ui: Topology - Fix up Default Allow and Permissive Intentions notices (…

…#11216) * ui: Default allow notices test (#11240)
hashicorp · Oct 12, 2021 · daec73e · daec73e
1 parent be667e2
commit daec73e
Show file tree

Hide file tree

Showing 11 changed files with 118 additions and 117 deletions.
diff --git a/.changelog/11216.txt b/.changelog/11216.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Topology - Fix up Default Allow and Permissive Intentions notices
+```
diff --git a/ui/packages/consul-ui/app/components/collapsible-notices/index.hbs b/ui/packages/consul-ui/app/components/collapsible-notices/index.hbs
@@ -1,10 +1,14 @@
-<div class="collapsible-notices {{if this.collapsed 'collapsed'}}">
-  <div class="notices">
-    {{yield}}
+{{#if @collapsible}}
+  <div class="collapsible-notices {{if this.collapsed 'collapsed'}}">
+    <div class="notices">
+      {{yield}}
+    </div>
+    {{#if this.collapsed}}
+    <button type="button" class="expand" {{on 'click' (set this 'collapsed' false)}}>{{t "components.app.collapsible-notices.expand"}}</button>
+    {{else}}
+    <button type="button" class="collapse" {{on 'click' (set this 'collapsed' true)}}>{{t "components.app.collapsible-notices.collapse"}}</button>
+    {{/if}}
   </div>
-{{#if this.collapsed}}
-  <button type="button" class="expand" {{on 'click' (set this 'collapsed' false)}}>{{t "components.app.collapsible-notices.expand"}}</button>
 {{else}}
-  <button type="button" class="collapse" {{on 'click' (set this 'collapsed' true)}}>{{t "components.app.collapsible-notices.collapse"}}</button>
+  {{yield}}
 {{/if}}
-</div>
diff --git a/ui/packages/consul-ui/app/components/topology-metrics/index.hbs b/ui/packages/consul-ui/app/components/topology-metrics/index.hbs
@@ -9,7 +9,7 @@
     {{did-update this.setHeight 'downstream-lines' @topology.Downstreams}}
   >
     <div>
-      <p>{{@dc}}</p>
+      <p>{{@dc.Name}}</p>
       <span>
         <Tooltip>
           Only showing downstreams within the current datacenter for {{@service.Service.Service}}.
@@ -19,7 +19,7 @@
   {{#each @topology.Downstreams as |item|}}
     <TopologyMetrics::Card
       @nspace={{@nspace}}
-      @dc={{@dc}}
+      @dc={{@dc.Name}}
       @service={{@service.Service}}
       @item={{item}}
       @hasMetricsProvider={{@hasMetricsProvider}}
@@ -50,7 +50,7 @@
       <TopologyMetrics::Series
         @nspace={{or @service.Service.Namespace 'default'}}
         @partition={{or service.Service.Partition 'default'}}
-        @dc={{@dc}}
+        @dc={{@dc.Name}}
         @service={{@service.Service.Service}}
         @protocol={{@topology.Protocol}}
         @noMetricsReason={{this.noMetricsReason}}
@@ -59,7 +59,7 @@
       <TopologyMetrics::Stats
         @nspace={{or @service.Service.Namespace 'default'}}
         @partition={{or service.Service.Partition 'default'}}
-        @dc={{@dc}}
+        @dc={{@dc.Name}}
         @endpoint='summary-for-service'
         @service={{@service.Service.Service}}
         @protocol={{@topology.Protocol}}
@@ -99,7 +99,7 @@
     {{/if}}
     {{#each upstreams as |item|}}
       <TopologyMetrics::Card
-        @dc={{@dc}}
+        @dc={{@dc.Name}}
         @item={{item}}
         @service={{@service.Service}}
       >

diff --git a/ui/packages/consul-ui/app/components/topology-metrics/index.js b/ui/packages/consul-ui/app/components/topology-metrics/index.js
@@ -69,9 +69,9 @@ export default class TopologyMetrics extends Component {
   get upstreams() {
     const upstreams = get(this.args.topology, 'Upstreams') || [];
     const items = [...upstreams];
-    const defaultAllow = get(this.args.topology, 'DefaultAllow');
-    const wildcardIntention = get(this.args.topology, 'WildcardIntention');
-    if (defaultAllow || wildcardIntention) {
+    const defaultACLPolicy = get(this.args.dc, 'DefaultACLPolicy');
+    const wildcardIntention = get(this.args.topology, 'wildcardIntention');
+    if (defaultACLPolicy === 'allow' || wildcardIntention) {
       items.push({
         Name: '* (All Services)',
         Datacenter: '',

diff --git a/ui/packages/consul-ui/app/helpers/collapsible-notices.js b/ui/packages/consul-ui/app/helpers/collapsible-notices.js
@@ -0,0 +1,9 @@
+import { helper } from '@ember/component/helper';
+
+export function collapsibleNotices(params, hash) {
+  // This filter will only return truthy items
+  const noticesCount = params.filter(Boolean).length;
+  return noticesCount > 2;
+}
+
+export default helper(collapsibleNotices);
diff --git a/ui/packages/consul-ui/app/models/topology.js b/ui/packages/consul-ui/app/models/topology.js
@@ -14,8 +14,6 @@ export default class Topology extends Model {
   @attr('string') Protocol;
   @attr('boolean') FilteredByACLs;
   @attr('boolean') TransparentProxy;
-  @attr('boolean') DefaultAllow;
-  @attr('boolean') WildcardIntention;
   @attr() Upstreams; // Service[]
   @attr() Downstreams; // Service[],
   @attr() meta; // {}
@@ -33,14 +31,19 @@ export default class Topology extends Model {
     return undefinedDownstream;
   }
 
-  @computed('FilteredByACL', 'DefaultAllow', 'WildcardIntention', 'notDefinedIntention')
-  get collapsible() {
-    if (this.DefaultAllow && this.FilteredByACLs && this.notDefinedIntention) {
-      return true;
-    } else if (this.WildcardIntention && this.FilteredByACLs && this.notDefinedIntention) {
-      return true;
-    }
+  @computed('Downstreams', 'Upstreams')
+  // A service has a wildcard intention if `Allowed == true`  and `HasExact = false`
+  // The Permissive Intention notice appears if at least one upstream or downstream has
+  // a wildcard intention
+  get wildcardIntention() {
+    const downstreamWildcard =
+      this.Downstreams.filter(item => !item.Intention.HasExact && item.Intention.Allowed).length !==
+      0;
 
-    return false;
+    const upstreamWildcard =
+      this.Upstreams.filter(item => !item.Intention.HasExact && item.Intention.Allowed).length !==
+      0;
+
+    return downstreamWildcard || upstreamWildcard;
   }
 }
diff --git a/ui/packages/consul-ui/app/templates/dc/services/show/topology.hbs b/ui/packages/consul-ui/app/templates/dc/services/show/topology.hbs
@@ -27,7 +27,7 @@ as |route|>
   loader.data
 as |nspace dc items topology|}}
   <div class="tab-section">
-    {{#if (and (eq topology.Upstreams.length 0) (eq topology.Downstreams.length 0) (not topology.DefaultAllow) (not topology.WildcardIntention))}}
+    {{#if (and (eq topology.Upstreams.length 0) (eq topology.Downstreams.length 0) (not-eq dc.DefaultACLPolicy 'allow') (not topology.wildcardIntention))}}
       <EmptyState>
         <BlockSlot @name="header">
           <h2>
@@ -46,84 +46,47 @@ as |nspace dc items topology|}}
         </BlockSlot>
       </EmptyState>
     {{else}}
-    {{#if topology.collapsible}}
-      <CollapsibleNotices>
-        {{#if topology.FilteredByACLs}}
-          <TopologyMetrics::Notice
-            data-test-notice='filtered-by-acls'
-            @type="info"
-            @for="limited-access"
-            @action={{false}}
-          />
-        {{/if}}
-        {{#if topology.DefaultAllow}}
-          <TopologyMetrics::Notice
-            data-test-notice='default-allow'
-            @type="warning"
-            @for="default-allow"
-            @internal={{true}}
-            @action={{true}}
-          />
-        {{/if}}
-        {{#if topology.WildcardIntention}}
-          <TopologyMetrics::Notice
-            data-test-notice='wildcard-intention'
-            @type="warning"
-            @for="wildcard-intention"
-            @internal={{true}}
-            @action={{true}}
-          />
-        {{/if}}
-        {{#if topology.notDefinedIntention}}
-          <TopologyMetrics::Notice
-            data-test-notice='not-defined-intention'
-            @type="warning"
-            @for="not-defined-intention"
-            @link="{{env 'CONSUL_DOCS_URL'}}/connect/registration/service-registration#upstreams"
-            @internal={{false}}
-            @action={{true}}
-          />
-        {{/if}}
-      </CollapsibleNotices>
-    {{else}}
-      {{#if topology.FilteredByACLs}}
-          <TopologyMetrics::Notice
-            data-test-notice='filtered-by-acls'
-            @type="info"
-            @for="limited-access"
-            @action={{false}}
-          />
-        {{/if}}
-        {{#if topology.DefaultAllow}}
-          <TopologyMetrics::Notice
-            data-test-notice='default-allow'
-            @type="warning"
-            @for="default-allow"
-            @internal={{true}}
-            @action={{true}}
-          />
-        {{/if}}
-        {{#if topology.WildcardIntention}}
-          <TopologyMetrics::Notice
-            data-test-notice='wildcard-intention'
-            @type="warning"
-            @for="wildcard-intention"
-            @internal={{true}}
-            @action={{true}}
-          />
-        {{/if}}
-        {{#if topology.notDefinedIntention}}
-          <TopologyMetrics::Notice
-            data-test-notice='not-defined-intention'
-            @type="warning"
-            @for="not-defined-intention"
-            @link="{{env 'CONSUL_DOCS_URL'}}/connect/registration/service-registration#upstreams"
-            @internal={{false}}
-            @action={{true}}
-          />
-        {{/if}}
-    {{/if}}
 
+  {{#let (collapsible-notices topology.FilteredByACLs (eq dc.DefaultACLPolicy 'allow') topology.wildcardIntention topology.notDefinedIntention) as |collapsible| }}
+    <CollapsibleNotices @collapsible={{collapsible}}>
+      {{#if topology.FilteredByACLs}}
+        <TopologyMetrics::Notice
+          data-test-notice='filtered-by-acls'
+          @type="info"
+          @for="limited-access"
+          @action={{false}}
+        />
+      {{/if}}
+      {{#if (eq dc.DefaultACLPolicy 'allow')}}
+        <TopologyMetrics::Notice
+          data-test-notice='default-allow'
+          @type="warning"
+          @for="default-allow"
+          @internal={{true}}
+          @action={{true}}
+        />
+      {{/if}}
+      {{#if topology.wildcardIntention}}
+        <TopologyMetrics::Notice
+          data-test-notice='wildcard-intention'
+          @type="warning"
+          @for="wildcard-intention"
+          @internal={{true}}
+          @action={{true}}
+        />
+      {{/if}}
+      {{#if topology.notDefinedIntention}}
+        <TopologyMetrics::Notice
+          data-test-notice='not-defined-intention'
+          @type="warning"
+          @for="not-defined-intention"
+          @link="{{env 'CONSUL_DOCS_URL'}}/connect/registration/service-registration#upstreams"
+          @internal={{false}}
+          @action={{true}}
+        />
+      {{/if}}
+    </CollapsibleNotices>
+  {{/let}}
     <DataSource
       @src={{uri '/${partition}/${nspace}/${dc}/ui-config'
         (hash
@@ -136,7 +99,7 @@ as |nspace dc items topology|}}
     {{#if config.data}}
       <TopologyMetrics
         @nspace={{nspace}}
-        @dc={{dc.Name}}
+        @dc={{dc}}
         @service={{items.firstObject}}
         @topology={{topology}}
 

diff --git a/ui/packages/consul-ui/mock-api/v1/catalog/.config b/ui/packages/consul-ui/mock-api/v1/catalog/.config
@@ -4,4 +4,4 @@
     "*":
       headers:
         response:
-          X-Consul-Default-Acl-Policy: ${fake.helpers.randomize(['allow', 'deny'])}
+          x-consul-default-acl-policy: ${env('CONSUL_ACL_POLICY', fake.helpers.randomize(['allow', 'deny']))}
diff --git a/ui/packages/consul-ui/mock-api/v1/internal/ui/service-topology/_ b/ui/packages/consul-ui/mock-api/v1/internal/ui/service-topology/_
@@ -53,17 +53,11 @@ ${
     fake.seed(index);
 
 
-    // Randomly make permissive intentions
-    const defaultAllow = fake.random.boolean();
-    const wildcardIntention = defaultAllow ? false : fake.random.boolean();
-
     return `
 {
   "Protocol": "${serviceProto}",
   "FilteredByACLs": ${fake.random.boolean()},
   "TransparentProxy": ${fake.random.boolean()},
-  "DefaultAllow": ${defaultAllow},
-  "WildcardIntention": ${wildcardIntention},
   "Upstreams": [
     ${
       upstreams.map((item, i) => {

diff --git a/ui/packages/consul-ui/tests/acceptance/dc/services/show/topology/tproxy.feature b/ui/packages/consul-ui/tests/acceptance/dc/services/show/topology/tproxy.feature
@@ -18,28 +18,50 @@ Feature: dc / services / show / topology / tproxy
         Name: web
         Kind: ~
     ---
-  Scenario: Deafult allow is set to true
+  Scenario: Default allow is set to true
     Given 1 topology model from yaml
     ---
       FilteredByACLs: false
       TransparentProxy: false
-      DefaultAllow: true
-      WildcardIntention: false
+      Downstreams:
+        - Name: db-1
+          Namespace: default
+          Datacenter: datacenter
+          Intention:
+            Allowed: false
+      Upstreams:
+        - Name: db-2
+          Namespace: default
+          Datacenter: datacenter
+          Intention:
+            Allowed: false
     ---
+    And the default ACL policy is "allow"
     When I visit the service page for yaml
     ---
       dc: datacenter
       service: web
     ---
     Then the url should be /datacenter/services/web/topology
     And I see the tabs.topologyTab.defaultAllowNotice object
-  Scenario: WildcardIntetions and FilteredByACLs are set to true
+  Scenario: A Downstream service has a wildcard intention
     Given 1 topology model from yaml
     ---
       FilteredByACLs: true
       TransparentProxy: false
-      DefaultAllow: false
-      WildcardIntention: true
+      Downstreams:
+        - Name: db-1
+          Namespace: default
+          Datacenter: datacenter
+          Intention:
+            Allowed: true
+            HasExact: false
+      Upstreams:
+        - Name: db-2
+          Namespace: default
+          Datacenter: datacenter
+          Intention:
+            Allowed: false
     ---
     When I visit the service page for yaml
     ---

diff --git a/ui/packages/consul-ui/tests/steps/doubles/model.js b/ui/packages/consul-ui/tests/steps/doubles/model.js
@@ -38,6 +38,9 @@ export default function(scenario, create, set, win = window, doc = document) {
     .given(['ACLs are disabled'], function() {
       doc.cookie = `CONSUL_ACLS_ENABLE=0`;
     })
+    .given(['the default ACL policy is "$policy"'], function(policy) {
+      set('CONSUL_ACL_POLICY', policy);
+    })
     .given(['a "$value" metrics provider'], function(value) {
       doc.cookie = `CONSUL_METRICS_PROXY_ENABLE=1`;
       doc.cookie = `CONSUL_METRICS_PROVIDER=${value}`;