From d9a23bb2fa5c6981bb904a63d2433e54557d9f43 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Tue, 17 Apr 2018 10:17:16 +0200 Subject: [PATCH 01/54] Track calls blocked by ACLs using metrics --- agent/local/state.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/agent/local/state.go b/agent/local/state.go index f19e88a766..cacbd6607b 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -10,6 +10,7 @@ import ( "sync/atomic" "time" + metrics "github.com/armon/go-metrics" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" @@ -842,6 +843,7 @@ func (l *State) deleteService(id string) error { // todo(fs): some backoff strategy might be a better solution l.services[id].InSync = true l.logger.Printf("[WARN] agent: Service %q deregistration blocked by ACLs", id) + metrics.IncrCounter([]string{"consul", "acl", "blocked", "service", "deregistration"}, 1) return nil default: @@ -879,6 +881,7 @@ func (l *State) deleteCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q deregistration blocked by ACLs", id) + metrics.IncrCounter([]string{"consul", "acl", "blocked", "check", "deregistration"}, 1) return nil default: @@ -949,6 +952,7 @@ func (l *State) syncService(id string) error { l.checks[check.CheckID].InSync = true } l.logger.Printf("[WARN] agent: Service %q registration blocked by ACLs", id) + metrics.IncrCounter([]string{"consul", "acl", "blocked", "service", "registration"}, 1) return nil default: @@ -994,6 +998,7 @@ func (l *State) syncCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q registration blocked by ACLs", id) + metrics.IncrCounter([]string{"consul", "acl", "blocked", "service", "registration"}, 1) return nil default: @@ -1025,6 +1030,7 @@ func (l *State) syncNodeInfo() error { // todo(fs): some backoff strategy might be a better solution l.nodeInfoInSync = true l.logger.Printf("[WARN] agent: Node info update blocked by ACLs") + metrics.IncrCounter([]string{"consul", "acl", "blocked", "node", "registration"}, 1) return nil default: From f13aa5ba9bbbaa2a3ba8ad539358bbfe3daba40c Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Wed, 18 Apr 2018 16:51:22 +0200 Subject: [PATCH 02/54] Added labels to improve new metric --- agent/local/state.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/agent/local/state.go b/agent/local/state.go index cacbd6607b..0e9b513e6c 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -843,7 +843,7 @@ func (l *State) deleteService(id string) error { // todo(fs): some backoff strategy might be a better solution l.services[id].InSync = true l.logger.Printf("[WARN] agent: Service %q deregistration blocked by ACLs", id) - metrics.IncrCounter([]string{"consul", "acl", "blocked", "service", "deregistration"}, 1) + metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "service", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: id}}) return nil default: @@ -881,7 +881,7 @@ func (l *State) deleteCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q deregistration blocked by ACLs", id) - metrics.IncrCounter([]string{"consul", "acl", "blocked", "check", "deregistration"}, 1) + metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "check", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: string(id)}}) return nil default: @@ -952,7 +952,7 @@ func (l *State) syncService(id string) error { l.checks[check.CheckID].InSync = true } l.logger.Printf("[WARN] agent: Service %q registration blocked by ACLs", id) - metrics.IncrCounter([]string{"consul", "acl", "blocked", "service", "registration"}, 1) + metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "service", "registration"}, 1, []metrics.Label{{Name: "id", Value: id}}) return nil default: @@ -998,7 +998,7 @@ func (l *State) syncCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q registration blocked by ACLs", id) - metrics.IncrCounter([]string{"consul", "acl", "blocked", "service", "registration"}, 1) + metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "check", "registration"}, 1, []metrics.Label{{Name: "check", Value: string(id)}}) return nil default: @@ -1030,7 +1030,7 @@ func (l *State) syncNodeInfo() error { // todo(fs): some backoff strategy might be a better solution l.nodeInfoInSync = true l.logger.Printf("[WARN] agent: Node info update blocked by ACLs") - metrics.IncrCounter([]string{"consul", "acl", "blocked", "node", "registration"}, 1) + metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "node", "registration"}, 1, []metrics.Label{{Name: "id", Value: string(l.config.NodeID)}, {Name: "name", Value: l.config.NodeName}}) return nil default: From 65d3a2b26e1fca8a54d88ec6390b48bd22d04dd7 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Wed, 18 Apr 2018 17:09:25 +0200 Subject: [PATCH 03/54] Fixed import --- agent/local/state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/local/state.go b/agent/local/state.go index 0e9b513e6c..c8505b2e27 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -10,7 +10,7 @@ import ( "sync/atomic" "time" - metrics "github.com/armon/go-metrics" + "github.com/armon/go-metrics" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" From 064f8ad1704943505a2d1f372555d25c0053c15b Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 8 Jun 2018 11:51:50 +0200 Subject: [PATCH 04/54] Removed consul prefix from metrics as requested by @kyhavlov --- agent/local/state.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/agent/local/state.go b/agent/local/state.go index c8505b2e27..da0b354505 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -843,7 +843,7 @@ func (l *State) deleteService(id string) error { // todo(fs): some backoff strategy might be a better solution l.services[id].InSync = true l.logger.Printf("[WARN] agent: Service %q deregistration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "service", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: id}}) + metrics.IncrCounterWithLabels([]string{"acl", "blocked", "service", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: id}}) return nil default: @@ -881,7 +881,7 @@ func (l *State) deleteCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q deregistration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "check", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: string(id)}}) + metrics.IncrCounterWithLabels([]string{"acl", "blocked", "check", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: string(id)}}) return nil default: @@ -952,7 +952,7 @@ func (l *State) syncService(id string) error { l.checks[check.CheckID].InSync = true } l.logger.Printf("[WARN] agent: Service %q registration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "service", "registration"}, 1, []metrics.Label{{Name: "id", Value: id}}) + metrics.IncrCounterWithLabels([]string{"acl", "blocked", "service", "registration"}, 1, []metrics.Label{{Name: "id", Value: id}}) return nil default: @@ -998,7 +998,7 @@ func (l *State) syncCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q registration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "check", "registration"}, 1, []metrics.Label{{Name: "check", Value: string(id)}}) + metrics.IncrCounterWithLabels([]string{"acl", "blocked", "check", "registration"}, 1, []metrics.Label{{Name: "check", Value: string(id)}}) return nil default: @@ -1030,7 +1030,7 @@ func (l *State) syncNodeInfo() error { // todo(fs): some backoff strategy might be a better solution l.nodeInfoInSync = true l.logger.Printf("[WARN] agent: Node info update blocked by ACLs") - metrics.IncrCounterWithLabels([]string{"consul", "acl", "blocked", "node", "registration"}, 1, []metrics.Label{{Name: "id", Value: string(l.config.NodeID)}, {Name: "name", Value: l.config.NodeName}}) + metrics.IncrCounterWithLabels([]string{"acl", "blocked", "node", "registration"}, 1, []metrics.Label{{Name: "id", Value: string(l.config.NodeID)}, {Name: "name", Value: l.config.NodeName}}) return nil default: From c83124a94cbf22664fa467008ed98f04658fd069 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 8 Jun 2018 11:56:46 +0200 Subject: [PATCH 05/54] Removed labels from new ACL denied metrics --- agent/local/state.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/agent/local/state.go b/agent/local/state.go index da0b354505..ba56636cda 100644 --- a/agent/local/state.go +++ b/agent/local/state.go @@ -843,7 +843,7 @@ func (l *State) deleteService(id string) error { // todo(fs): some backoff strategy might be a better solution l.services[id].InSync = true l.logger.Printf("[WARN] agent: Service %q deregistration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"acl", "blocked", "service", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: id}}) + metrics.IncrCounter([]string{"acl", "blocked", "service", "deregistration"}, 1) return nil default: @@ -881,7 +881,7 @@ func (l *State) deleteCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q deregistration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"acl", "blocked", "check", "deregistration"}, 1, []metrics.Label{{Name: "id", Value: string(id)}}) + metrics.IncrCounter([]string{"acl", "blocked", "check", "deregistration"}, 1) return nil default: @@ -952,7 +952,7 @@ func (l *State) syncService(id string) error { l.checks[check.CheckID].InSync = true } l.logger.Printf("[WARN] agent: Service %q registration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"acl", "blocked", "service", "registration"}, 1, []metrics.Label{{Name: "id", Value: id}}) + metrics.IncrCounter([]string{"acl", "blocked", "service", "registration"}, 1) return nil default: @@ -998,7 +998,7 @@ func (l *State) syncCheck(id types.CheckID) error { // todo(fs): some backoff strategy might be a better solution l.checks[id].InSync = true l.logger.Printf("[WARN] agent: Check %q registration blocked by ACLs", id) - metrics.IncrCounterWithLabels([]string{"acl", "blocked", "check", "registration"}, 1, []metrics.Label{{Name: "check", Value: string(id)}}) + metrics.IncrCounter([]string{"acl", "blocked", "check", "registration"}, 1) return nil default: @@ -1030,7 +1030,7 @@ func (l *State) syncNodeInfo() error { // todo(fs): some backoff strategy might be a better solution l.nodeInfoInSync = true l.logger.Printf("[WARN] agent: Node info update blocked by ACLs") - metrics.IncrCounterWithLabels([]string{"acl", "blocked", "node", "registration"}, 1, []metrics.Label{{Name: "id", Value: string(l.config.NodeID)}, {Name: "name", Value: l.config.NodeName}}) + metrics.IncrCounter([]string{"acl", "blocked", "node", "registration"}, 1) return nil default: From 44deab454d3974542b2fda4aa2e7649fb7b2a4f5 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Mon, 25 Jun 2018 12:11:01 +0100 Subject: [PATCH 06/54] Reset `` after every test, back to its original static value --- ui-v2/tests/acceptance/startup.feature | 1 - ui-v2/tests/helpers/yadda-annotations.js | 15 +++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/ui-v2/tests/acceptance/startup.feature b/ui-v2/tests/acceptance/startup.feature index 70e54705ca..a847fef3d6 100644 --- a/ui-v2/tests/acceptance/startup.feature +++ b/ui-v2/tests/acceptance/startup.feature @@ -3,7 +3,6 @@ Feature: startup In order to give users an indication as early as possible that they are at the right place As a user I should be able to see a startup logo -@ignore Scenario: When loading the index.html file into a browser Given 1 datacenter model with the value "dc-1" Then the url should be '' diff --git a/ui-v2/tests/helpers/yadda-annotations.js b/ui-v2/tests/helpers/yadda-annotations.js index 7c972601e4..43cc67853b 100644 --- a/ui-v2/tests/helpers/yadda-annotations.js +++ b/ui-v2/tests/helpers/yadda-annotations.js @@ -3,6 +3,18 @@ import { skip } from 'qunit'; import { setupApplicationTest, setupRenderingTest, setupTest } from 'ember-qunit'; import api from 'consul-ui/tests/helpers/api'; +const staticClassList = [...document.documentElement.classList]; +function reset() { + window.localStorage.clear(); + api.server.reset(); + const list = document.documentElement.classList; + while (list.length > 0) { + list.remove(list.item(0)); + } + staticClassList.forEach(function(item) { + list.add(item); + }); +} // this logic could be anything, but in this case... // if @ignore, then return skip (for backwards compatibility) // if have annotations in config, then only run those that have a matching annotation @@ -64,8 +76,7 @@ function setupScenario(featureAnnotations, scenarioAnnotations) { } return function(model) { model.afterEach(function() { - window.localStorage.clear(); - api.server.reset(); + reset(); }); }; // return setupFn; From 8e7e224f3fdb69497e000c25c5c109b6aa1d6b1f Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 26 Jun 2018 10:48:26 +0100 Subject: [PATCH 07/54] Encode all the hexcodes --- ui-v2/app/styles/components/icons.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui-v2/app/styles/components/icons.scss b/ui-v2/app/styles/components/icons.scss index 5b4c139302..e12d1fae52 100644 --- a/ui-v2/app/styles/components/icons.scss +++ b/ui-v2/app/styles/components/icons.scss @@ -152,18 +152,18 @@ } %with-right-arrow-grey { @extend %pseudo-icon; - background-image: url('data:image/svg+xml;charset=UTF-8,'); + background-image: url('data:image/svg+xml;charset=UTF-8,'); } %with-deny-icon { @extend %pseudo-icon; - background-image: url('data:image/svg+xml;charset=UTF-8,'); + background-image: url('data:image/svg+xml;charset=UTF-8,'); width: 16px; height: 16px; background-color: transparent; } %with-deny-icon-grey { @extend %pseudo-icon; - background-image: url('data:image/svg+xml;charset=UTF-8,'); + background-image: url('data:image/svg+xml;charset=UTF-8,'); } %with-deny::before { @extend %with-deny-icon; From 9406ca1c959e591299351dfaf26fbf0c26dd71c0 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Thu, 28 Jun 2018 09:06:14 +0200 Subject: [PATCH 08/54] Only send one single ACL cache refresh across network when TTL is over It will allow the following: * when connectivity is limited (saturated linnks between DCs), only one single request to refresh an ACL will be sent to ACL master DC instead of statcking ACL refresh queries * when extend-cache is used for ACL, do not wait for result, but refresh the ACL asynchronously, so no delay is not impacting slave DC * When extend-cache is not used, keep the existing blocking mechanism, but only send a single refresh request. This will fix https://github.com/hashicorp/consul/issues/3524 --- agent/consul/acl.go | 79 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 7 deletions(-) diff --git a/agent/consul/acl.go b/agent/consul/acl.go index ce3282b40e..0bf1dbb4cd 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "os" + "sync" "time" "github.com/armon/go-metrics" @@ -116,6 +117,9 @@ type aclCache struct { // local is a function used to look for an ACL locally if replication is // enabled. This will be nil if replication isn't enabled. local acl.FaultFunc + + fetchMutex sync.RWMutex + fetchMap map[string][]chan (RemoteACLResult) } // newACLCache returns a new non-authoritative cache for ACLs. This is used for @@ -146,6 +150,11 @@ func newACLCache(conf *Config, logger *log.Logger, rpc rpcFn, local acl.FaultFun return cache, nil } +type RemoteACLResult struct { + result acl.ACL + err error +} + // lookupACL is used when we are non-authoritative, and need to resolve an ACL. func (c *aclCache) lookupACL(id, authDC string) (acl.ACL, error) { // Check the cache for the ACL. @@ -161,8 +170,22 @@ func (c *aclCache) lookupACL(id, authDC string) (acl.ACL, error) { return cached.ACL, nil } metrics.IncrCounter([]string{"acl", "cache_miss"}, 1) + res := c.lookupACLRemote(id, authDC, cached) + return res.result, res.err +} - // Attempt to refresh the policy from the ACL datacenter via an RPC. +func (c *aclCache) fireResult(id string, theACL acl.ACL, err error) { + c.fetchMutex.Lock() + channels := c.fetchMap[id] + delete(c.fetchMap, id) + c.fetchMutex.Unlock() + for _, cx := range channels { + cx <- RemoteACLResult{theACL, err} + close(cx) + } +} + +func (c *aclCache) loadACLInChan(id, authDC string, cached *aclCacheEntry) { args := structs.ACLPolicyRequest{ Datacenter: authDC, ACL: id, @@ -173,13 +196,21 @@ func (c *aclCache) lookupACL(id, authDC string) (acl.ACL, error) { var reply structs.ACLPolicy err := c.rpc("ACL.GetPolicy", &args, &reply) if err == nil { - return c.useACLPolicy(id, authDC, cached, &reply) + theACL, theError := c.useACLPolicy(id, authDC, cached, &reply) + if cached != nil && theACL != nil { + cached.ACL = theACL + cached.ETag = reply.ETag + cached.Expires = time.Now().Add(c.config.ACLTTL) + } + c.fireResult(id, theACL, theError) + return } // Check for not-found, which will cause us to bail immediately. For any // other error we report it in the logs but can continue. if acl.IsErrNotFound(err) { - return nil, acl.ErrNotFound + c.fireResult(id, nil, acl.ErrNotFound) + return } c.logger.Printf("[ERR] consul.acl: Failed to get policy from ACL datacenter: %v", err) @@ -227,24 +258,58 @@ func (c *aclCache) lookupACL(id, authDC string) (acl.ACL, error) { reply.TTL = c.config.ACLTTL reply.Parent = parent reply.Policy = policy - return c.useACLPolicy(id, authDC, cached, &reply) + theACL, theError := c.useACLPolicy(id, authDC, cached, &reply) + if cached != nil && theACL != nil { + cached.ACL = theACL + cached.ETag = reply.ETag + cached.Expires = time.Now().Add(c.config.ACLTTL) + } + c.fireResult(id, theACL, theError) + return } ACL_DOWN: // Unable to refresh, apply the down policy. switch c.config.ACLDownPolicy { case "allow": - return acl.AllowAll(), nil + c.fireResult(id, acl.AllowAll(), nil) + return case "extend-cache": if cached != nil { - return cached.ACL, nil + c.fireResult(id, cached.ACL, nil) + return } fallthrough default: - return acl.DenyAll(), nil + c.fireResult(id, acl.DenyAll(), nil) + return } } +func (c *aclCache) lookupACLRemote(id, authDC string, cached *aclCacheEntry) RemoteACLResult { + // Attempt to refresh the policy from the ACL datacenter via an RPC. + myChan := make(chan RemoteACLResult) + mustWaitForResult := cached == nil || c.config.ACLDownPolicy != "extend-cache" + c.fetchMutex.Lock() + clients, ok := c.fetchMap[id] + if !ok { + clients = make([]chan RemoteACLResult, 16) + } + if mustWaitForResult { + c.fetchMap[id] = append(clients, myChan) + } + c.fetchMutex.Unlock() + + if !ok { + go c.loadACLInChan(id, authDC, cached) + } + if !mustWaitForResult { + return RemoteACLResult{cached.ACL, nil} + } + res := <-myChan + return res +} + // useACLPolicy handles an ACLPolicy response func (c *aclCache) useACLPolicy(id, authDC string, cached *aclCacheEntry, p *structs.ACLPolicy) (acl.ACL, error) { // Check if we can used the cached policy From bfc83ce045ee6c2ef56079e0e5ecb4fda931ed05 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 29 Jun 2018 10:59:36 +0200 Subject: [PATCH 09/54] Updated ACL guide --- website/source/docs/guides/acl.html.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/website/source/docs/guides/acl.html.md b/website/source/docs/guides/acl.html.md index a78f60b55e..f6aad52bce 100644 --- a/website/source/docs/guides/acl.html.md +++ b/website/source/docs/guides/acl.html.md @@ -1061,6 +1061,10 @@ and the [`acl_down_policy`](/docs/agent/options.html#acl_down_policy) is set to "extend-cache", tokens will be resolved during the outage using the replicated set of ACLs. An [ACL replication status](/api/acl.html#acl_replication_status) endpoint is available to monitor the health of the replication process. +Also note that in recent versions of Consul (greater than 1.2.0), using +`acl_down_policy = "extend-cache"` refreshes token asynchronously when an ACL is +already cached and is expired. It allows to avoid having issues when connectivity with +the authoritative is not completely broken, but very slow. Locally-resolved ACLs will be cached using the [`acl_ttl`](/docs/agent/options.html#acl_ttl) setting of the non-authoritative datacenter, so these entries may persist in the From abde81a3e759b99aefab232d78fb871d9688f532 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Sun, 1 Jul 2018 12:50:53 +0200 Subject: [PATCH 10/54] Added async-cache with similar behaviour as extend-cache but asynchronously --- agent/acl.go | 1 + agent/acl_test.go | 133 ++++++++++++++++++++------------------- agent/config/runtime.go | 4 +- agent/consul/acl.go | 14 +++-- agent/consul/acl_test.go | 132 +++++++++++++++++++------------------- agent/consul/config.go | 6 +- 6 files changed, 153 insertions(+), 137 deletions(-) diff --git a/agent/acl.go b/agent/acl.go index e266feafa8..1143da97cc 100644 --- a/agent/acl.go +++ b/agent/acl.go @@ -104,6 +104,7 @@ func newACLManager(config *config.RuntimeConfig) (*aclManager, error) { case "deny": down = acl.DenyAll() case "extend-cache": + case "async-cache": // Leave the down policy as nil to signal this. default: return nil, fmt.Errorf("invalid ACL down policy %q", config.ACLDownPolicy) diff --git a/agent/acl_test.go b/agent/acl_test.go index 8b16269981..0932dc5254 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -274,79 +274,82 @@ func TestACL_Down_Allow(t *testing.T) { func TestACL_Down_Extend(t *testing.T) { t.Parallel() - a := NewTestAgent(t.Name(), TestACLConfig()+` - acl_down_policy = "extend-cache" + aclExtendPolicies := []string{"extend-cache", "async-cache"} + for _, aclDownPolicy := range aclExtendPolicies { + a := NewTestAgent(t.Name(), TestACLConfig()+` + acl_down_policy = "`+aclDownPolicy+`" acl_enforce_version_8 = true `) - defer a.Shutdown() + defer a.Shutdown() - m := MockServer{ - // Populate the cache for one of the tokens. - getPolicyFn: func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { - *reply = structs.ACLPolicy{ - Parent: "allow", - Policy: &rawacl.Policy{ - Agents: []*rawacl.AgentPolicy{ - &rawacl.AgentPolicy{ - Node: a.config.NodeName, - Policy: "read", + m := MockServer{ + // Populate the cache for one of the tokens. + getPolicyFn: func(req *structs.ACLPolicyRequest, reply *structs.ACLPolicy) error { + *reply = structs.ACLPolicy{ + Parent: "allow", + Policy: &rawacl.Policy{ + Agents: []*rawacl.AgentPolicy{ + &rawacl.AgentPolicy{ + Node: a.config.NodeName, + Policy: "read", + }, }, }, - }, - } - return nil - }, - } - if err := a.registerEndpoint("ACL", &m); err != nil { - t.Fatalf("err: %v", err) - } + } + return nil + }, + } + if err := a.registerEndpoint("ACL", &m); err != nil { + t.Fatalf("err: %v", err) + } - acl, err := a.resolveToken("yep") - if err != nil { - t.Fatalf("err: %v", err) - } - if acl == nil { - t.Fatalf("should not be nil") - } - if !acl.AgentRead(a.config.NodeName) { - t.Fatalf("should allow") - } - if acl.AgentWrite(a.config.NodeName) { - t.Fatalf("should deny") - } + acl, err := a.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(a.config.NodeName) { + t.Fatalf("should allow") + } + if acl.AgentWrite(a.config.NodeName) { + t.Fatalf("should deny") + } - // Now take down ACLs and make sure a new token fails to resolve. - m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { - return fmt.Errorf("ACLs are broken") - } - acl, err = a.resolveToken("nope") - if err != nil { - t.Fatalf("err: %v", err) - } - if acl == nil { - t.Fatalf("should not be nil") - } - if acl.AgentRead(a.config.NodeName) { - t.Fatalf("should deny") - } - if acl.AgentWrite(a.config.NodeName) { - t.Fatalf("should deny") - } + // Now take down ACLs and make sure a new token fails to resolve. + m.getPolicyFn = func(*structs.ACLPolicyRequest, *structs.ACLPolicy) error { + return fmt.Errorf("ACLs are broken") + } + acl, err = a.resolveToken("nope") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if acl.AgentRead(a.config.NodeName) { + t.Fatalf("should deny") + } + if acl.AgentWrite(a.config.NodeName) { + t.Fatalf("should deny") + } - // Read the token from the cache while ACLs are broken, which should - // extend. - acl, err = a.resolveToken("yep") - if err != nil { - t.Fatalf("err: %v", err) - } - if acl == nil { - t.Fatalf("should not be nil") - } - if !acl.AgentRead(a.config.NodeName) { - t.Fatalf("should allow") - } - if acl.AgentWrite(a.config.NodeName) { - t.Fatalf("should deny") + // Read the token from the cache while ACLs are broken, which should + // extend. + acl, err = a.resolveToken("yep") + if err != nil { + t.Fatalf("err: %v", err) + } + if acl == nil { + t.Fatalf("should not be nil") + } + if !acl.AgentRead(a.config.NodeName) { + t.Fatalf("should allow") + } + if acl.AgentWrite(a.config.NodeName) { + t.Fatalf("should deny") + } } } diff --git a/agent/config/runtime.go b/agent/config/runtime.go index 9674d14d5f..05ad8796ca 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -94,8 +94,10 @@ type RuntimeConfig struct { // ACL's to be used to service requests. This // is the default. If the ACL is not in the cache, // this acts like deny. + // * async-cache - Same behaviour as extend-cache, but perform ACL + // Lookups asynchronously when cache TTL is expired. // - // hcl: acl_down_policy = ("allow"|"deny"|"extend-cache") + // hcl: acl_down_policy = ("allow"|"deny"|"extend-cache"|"async-cache") ACLDownPolicy string // ACLEnforceVersion8 is used to gate a set of ACL policy features that diff --git a/agent/consul/acl.go b/agent/consul/acl.go index 0bf1dbb4cd..23bdbff356 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -146,10 +146,12 @@ func newACLCache(conf *Config, logger *log.Logger, rpc rpcFn, local acl.FaultFun if err != nil { return nil, fmt.Errorf("Failed to create ACL policy cache: %v", err) } + cache.fetchMap = make(map[string][]chan (RemoteACLResult)) return cache, nil } +// Result Type returned when fetching Remote ACLs asynchronously type RemoteACLResult struct { result acl.ACL err error @@ -179,8 +181,9 @@ func (c *aclCache) fireResult(id string, theACL acl.ACL, err error) { channels := c.fetchMap[id] delete(c.fetchMap, id) c.fetchMutex.Unlock() + aclResult := RemoteACLResult{theACL, err} for _, cx := range channels { - cx <- RemoteACLResult{theACL, err} + cx <- aclResult close(cx) } } @@ -231,7 +234,7 @@ func (c *aclCache) loadACLInChan(id, authDC string, cached *aclCacheEntry) { // local ACL fault function is registered to query replicated ACL data, // and the user's policy allows it, we will try locally before we give // up. - if c.local != nil && c.config.ACLDownPolicy == "extend-cache" { + if c.local != nil && (c.config.ACLDownPolicy == "extend-cache" || c.config.ACLDownPolicy == "async-cache") { parent, rules, err := c.local(id) if err != nil { // We don't make an exception here for ACLs that aren't @@ -274,6 +277,7 @@ ACL_DOWN: case "allow": c.fireResult(id, acl.AllowAll(), nil) return + case "async-cache": case "extend-cache": if cached != nil { c.fireResult(id, cached.ACL, nil) @@ -289,11 +293,11 @@ ACL_DOWN: func (c *aclCache) lookupACLRemote(id, authDC string, cached *aclCacheEntry) RemoteACLResult { // Attempt to refresh the policy from the ACL datacenter via an RPC. myChan := make(chan RemoteACLResult) - mustWaitForResult := cached == nil || c.config.ACLDownPolicy != "extend-cache" + mustWaitForResult := cached == nil || c.config.ACLDownPolicy != "async-cache" c.fetchMutex.Lock() clients, ok := c.fetchMap[id] - if !ok { - clients = make([]chan RemoteACLResult, 16) + if !ok || clients == nil { + clients = make([]chan RemoteACLResult, 0) } if mustWaitForResult { c.fetchMap[id] = append(clients, myChan) diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index ace1284a86..fd08b54ab8 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -508,78 +508,82 @@ func TestACL_DownPolicy_Allow(t *testing.T) { func TestACL_DownPolicy_ExtendCache(t *testing.T) { t.Parallel() - dir1, s1 := testServerWithConfig(t, func(c *Config) { - c.ACLDatacenter = "dc1" - c.ACLTTL = 0 - c.ACLDownPolicy = "extend-cache" - c.ACLMasterToken = "root" - }) - defer os.RemoveAll(dir1) - defer s1.Shutdown() - client := rpcClient(t, s1) - defer client.Close() + aclExtendPolicies := []string{"extend-cache", "async-cache"} //"async-cache" - dir2, s2 := testServerWithConfig(t, func(c *Config) { - c.ACLDatacenter = "dc1" // Enable ACLs! - c.ACLTTL = 0 - c.ACLDownPolicy = "extend-cache" - c.Bootstrap = false // Disable bootstrap - }) - defer os.RemoveAll(dir2) - defer s2.Shutdown() + for _, aclDownPolicy := range aclExtendPolicies { + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLTTL = 0 + c.ACLDownPolicy = aclDownPolicy + c.ACLMasterToken = "root" + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + client := rpcClient(t, s1) + defer client.Close() - // Try to join - joinLAN(t, s2, s1) - retry.Run(t, func(r *retry.R) { r.Check(wantRaft([]*Server{s1, s2})) }) + dir2, s2 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" // Enable ACLs! + c.ACLTTL = 0 + c.ACLDownPolicy = aclDownPolicy + c.Bootstrap = false // Disable bootstrap + }) + defer os.RemoveAll(dir2) + defer s2.Shutdown() - testrpc.WaitForLeader(t, s1.RPC, "dc1") + // Try to join + joinLAN(t, s2, s1) + retry.Run(t, func(r *retry.R) { r.Check(wantRaft([]*Server{s1, s2})) }) - // Create a new token - arg := structs.ACLRequest{ - Datacenter: "dc1", - Op: structs.ACLSet, - ACL: structs.ACL{ - Name: "User token", - Type: structs.ACLTypeClient, - Rules: testACLPolicy, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, - } - var id string - if err := s1.RPC("ACL.Apply", &arg, &id); err != nil { - t.Fatalf("err: %v", err) - } + testrpc.WaitForLeader(t, s1.RPC, "dc1") - // find the non-authoritative server - var nonAuth *Server - var auth *Server - if !s1.IsLeader() { - nonAuth = s1 - auth = s2 - } else { - nonAuth = s2 - auth = s1 - } + // Create a new token + arg := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: testACLPolicy, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + var id string + if err := s1.RPC("ACL.Apply", &arg, &id); err != nil { + t.Fatalf("err: %v", err) + } - // Warm the caches - aclR, err := nonAuth.resolveToken(id) - if err != nil { - t.Fatalf("err: %v", err) - } - if aclR == nil { - t.Fatalf("bad acl: %#v", aclR) - } + // find the non-authoritative server + var nonAuth *Server + var auth *Server + if !s1.IsLeader() { + nonAuth = s1 + auth = s2 + } else { + nonAuth = s2 + auth = s1 + } - // Kill the authoritative server - auth.Shutdown() + // Warm the caches + aclR, err := nonAuth.resolveToken(id) + if err != nil { + t.Fatalf("err: %v", err) + } + if aclR == nil { + t.Fatalf("bad acl: %#v", aclR) + } - // Token should resolve into cached copy - aclR2, err := nonAuth.resolveToken(id) - if err != nil { - t.Fatalf("err: %v", err) - } - if aclR2 != aclR { - t.Fatalf("bad acl: %#v", aclR) + // Kill the authoritative server + auth.Shutdown() + + // Token should resolve into cached copy + aclR2, err := nonAuth.resolveToken(id) + if err != nil { + t.Fatalf("err: %v", err) + } + if aclR2 != aclR { + t.Fatalf("bad acl: %#v", aclR) + } } } diff --git a/agent/consul/config.go b/agent/consul/config.go index 29a5245319..570a07c163 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -235,8 +235,9 @@ type Config struct { // ACLDownPolicy controls the behavior of ACLs if the ACLDatacenter // cannot be contacted. It can be either "deny" to deny all requests, - // or "extend-cache" which ignores the ACLCacheInterval and uses - // cached policies. If a policy is not in the cache, it acts like deny. + // "extend-cache" or "async-cache" which ignores the ACLCacheInterval and + // uses cached policies. + // If a policy is not in the cache, it acts like deny. // "allow" can be used to allow all requests. This is not recommended. ACLDownPolicy string @@ -378,6 +379,7 @@ func (c *Config) CheckACL() error { switch c.ACLDownPolicy { case "allow": case "deny": + case "async-cache": case "extend-cache": default: return fmt.Errorf("Unsupported down ACL policy: %s", c.ACLDownPolicy) From 1e7665c0d58111ee7f4d1fcd4d6fbd7b26c0ec82 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Sun, 1 Jul 2018 20:00:20 +0200 Subject: [PATCH 11/54] Updated documentation and adding more test case for async-cache --- agent/consul/acl_test.go | 198 +++++++++++----------- website/source/docs/agent/options.html.md | 6 +- website/source/docs/guides/acl.html.md | 7 +- 3 files changed, 109 insertions(+), 102 deletions(-) diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index fd08b54ab8..45048f80a1 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -589,116 +589,120 @@ func TestACL_DownPolicy_ExtendCache(t *testing.T) { func TestACL_Replication(t *testing.T) { t.Parallel() - dir1, s1 := testServerWithConfig(t, func(c *Config) { - c.ACLDatacenter = "dc1" - c.ACLMasterToken = "root" - }) - defer os.RemoveAll(dir1) - defer s1.Shutdown() - client := rpcClient(t, s1) - defer client.Close() + aclExtendPolicies := []string{"extend-cache", "async-cache"} //"async-cache" - dir2, s2 := testServerWithConfig(t, func(c *Config) { - c.Datacenter = "dc2" - c.ACLDatacenter = "dc1" - c.ACLDefaultPolicy = "deny" - c.ACLDownPolicy = "extend-cache" - c.EnableACLReplication = true - c.ACLReplicationInterval = 10 * time.Millisecond - c.ACLReplicationApplyLimit = 1000000 - }) - s2.tokens.UpdateACLReplicationToken("root") - defer os.RemoveAll(dir2) - defer s2.Shutdown() + for _, aclDownPolicy := range aclExtendPolicies { + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLMasterToken = "root" + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + client := rpcClient(t, s1) + defer client.Close() - dir3, s3 := testServerWithConfig(t, func(c *Config) { - c.Datacenter = "dc3" - c.ACLDatacenter = "dc1" - c.ACLDownPolicy = "deny" - c.EnableACLReplication = true - c.ACLReplicationInterval = 10 * time.Millisecond - c.ACLReplicationApplyLimit = 1000000 - }) - s3.tokens.UpdateACLReplicationToken("root") - defer os.RemoveAll(dir3) - defer s3.Shutdown() + dir2, s2 := testServerWithConfig(t, func(c *Config) { + c.Datacenter = "dc2" + c.ACLDatacenter = "dc1" + c.ACLDefaultPolicy = "deny" + c.ACLDownPolicy = aclDownPolicy + c.EnableACLReplication = true + c.ACLReplicationInterval = 10 * time.Millisecond + c.ACLReplicationApplyLimit = 1000000 + }) + s2.tokens.UpdateACLReplicationToken("root") + defer os.RemoveAll(dir2) + defer s2.Shutdown() - // Try to join. - joinWAN(t, s2, s1) - joinWAN(t, s3, s1) - testrpc.WaitForLeader(t, s1.RPC, "dc1") - testrpc.WaitForLeader(t, s1.RPC, "dc2") - testrpc.WaitForLeader(t, s1.RPC, "dc3") + dir3, s3 := testServerWithConfig(t, func(c *Config) { + c.Datacenter = "dc3" + c.ACLDatacenter = "dc1" + c.ACLDownPolicy = "deny" + c.EnableACLReplication = true + c.ACLReplicationInterval = 10 * time.Millisecond + c.ACLReplicationApplyLimit = 1000000 + }) + s3.tokens.UpdateACLReplicationToken("root") + defer os.RemoveAll(dir3) + defer s3.Shutdown() - // Create a new token. - arg := structs.ACLRequest{ - Datacenter: "dc1", - Op: structs.ACLSet, - ACL: structs.ACL{ - Name: "User token", - Type: structs.ACLTypeClient, - Rules: testACLPolicy, - }, - WriteRequest: structs.WriteRequest{Token: "root"}, - } - var id string - if err := s1.RPC("ACL.Apply", &arg, &id); err != nil { - t.Fatalf("err: %v", err) - } - // Wait for replication to occur. - retry.Run(t, func(r *retry.R) { - _, acl, err := s2.fsm.State().ACLGet(nil, id) + // Try to join. + joinWAN(t, s2, s1) + joinWAN(t, s3, s1) + testrpc.WaitForLeader(t, s1.RPC, "dc1") + testrpc.WaitForLeader(t, s1.RPC, "dc2") + testrpc.WaitForLeader(t, s1.RPC, "dc3") + + // Create a new token. + arg := structs.ACLRequest{ + Datacenter: "dc1", + Op: structs.ACLSet, + ACL: structs.ACL{ + Name: "User token", + Type: structs.ACLTypeClient, + Rules: testACLPolicy, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + } + var id string + if err := s1.RPC("ACL.Apply", &arg, &id); err != nil { + t.Fatalf("err: %v", err) + } + // Wait for replication to occur. + retry.Run(t, func(r *retry.R) { + _, acl, err := s2.fsm.State().ACLGet(nil, id) + if err != nil { + r.Fatal(err) + } + if acl == nil { + r.Fatal(nil) + } + _, acl, err = s3.fsm.State().ACLGet(nil, id) + if err != nil { + r.Fatal(err) + } + if acl == nil { + r.Fatal(nil) + } + }) + + // Kill the ACL datacenter. + s1.Shutdown() + + // Token should resolve on s2, which has replication + extend-cache. + acl, err := s2.resolveToken(id) if err != nil { - r.Fatal(err) + t.Fatalf("err: %v", err) } if acl == nil { - r.Fatal(nil) + t.Fatalf("missing acl") } - _, acl, err = s3.fsm.State().ACLGet(nil, id) + + // Check the policy + if acl.KeyRead("bar") { + t.Fatalf("unexpected read") + } + if !acl.KeyRead("foo/test") { + t.Fatalf("unexpected failed read") + } + + // Although s3 has replication, and we verified that the ACL is there, + // it can not be used because of the down policy. + acl, err = s3.resolveToken(id) if err != nil { - r.Fatal(err) + t.Fatalf("err: %v", err) } if acl == nil { - r.Fatal(nil) + t.Fatalf("missing acl") } - }) - // Kill the ACL datacenter. - s1.Shutdown() - - // Token should resolve on s2, which has replication + extend-cache. - acl, err := s2.resolveToken(id) - if err != nil { - t.Fatalf("err: %v", err) - } - if acl == nil { - t.Fatalf("missing acl") - } - - // Check the policy - if acl.KeyRead("bar") { - t.Fatalf("unexpected read") - } - if !acl.KeyRead("foo/test") { - t.Fatalf("unexpected failed read") - } - - // Although s3 has replication, and we verified that the ACL is there, - // it can not be used because of the down policy. - acl, err = s3.resolveToken(id) - if err != nil { - t.Fatalf("err: %v", err) - } - if acl == nil { - t.Fatalf("missing acl") - } - - // Check the policy. - if acl.KeyRead("bar") { - t.Fatalf("unexpected read") - } - if acl.KeyRead("foo/test") { - t.Fatalf("unexpected read") + // Check the policy. + if acl.KeyRead("bar") { + t.Fatalf("unexpected read") + } + if acl.KeyRead("foo/test") { + t.Fatalf("unexpected read") + } } } diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index 4e2ecb3033..65402c2caf 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -496,11 +496,13 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass to enable ACL support. * `acl_down_policy` - Either - "allow", "deny" or "extend-cache"; "extend-cache" is the default. In the case that the + "allow", "deny", "extend-cache" or "async-cache"; "extend-cache" is the default. In the case that the policy for a token cannot be read from the [`acl_datacenter`](#acl_datacenter) or leader node, the down policy is applied. In "allow" mode, all actions are permitted, "deny" restricts all operations, and "extend-cache" allows any cached ACLs to be used, ignoring their TTL - values. If a non-cached ACL is used, "extend-cache" acts like "deny". + values. If a non-cached ACL is used, "extend-cache" acts like "deny". "async-cache" acts the same + way as "extend-cache" but performs updates asynchronously when ACL is present but its TTL is + expired. * `acl_agent_master_token` - Used to access agent endpoints that require agent read diff --git a/website/source/docs/guides/acl.html.md b/website/source/docs/guides/acl.html.md index f6aad52bce..c25666ab94 100644 --- a/website/source/docs/guides/acl.html.md +++ b/website/source/docs/guides/acl.html.md @@ -1062,9 +1062,10 @@ is set to "extend-cache", tokens will be resolved during the outage using the replicated set of ACLs. An [ACL replication status](/api/acl.html#acl_replication_status) endpoint is available to monitor the health of the replication process. Also note that in recent versions of Consul (greater than 1.2.0), using -`acl_down_policy = "extend-cache"` refreshes token asynchronously when an ACL is -already cached and is expired. It allows to avoid having issues when connectivity with -the authoritative is not completely broken, but very slow. +`acl_down_policy = "async-cache"` refreshes token asynchronously when an ACL is +already cached and is expired while similar semantics than "extend-cache". +It allows to avoid having issues when connectivity with the authoritative is not completely +broken, but very slow. Locally-resolved ACLs will be cached using the [`acl_ttl`](/docs/agent/options.html#acl_ttl) setting of the non-authoritative datacenter, so these entries may persist in the From 0f1735634d1f9d522be336bc302a4684728dab67 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Sun, 1 Jul 2018 23:37:40 +0200 Subject: [PATCH 12/54] Improve doc for async-cache --- website/source/docs/agent/options.html.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index 65402c2caf..06f071dd04 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -500,9 +500,10 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass policy for a token cannot be read from the [`acl_datacenter`](#acl_datacenter) or leader node, the down policy is applied. In "allow" mode, all actions are permitted, "deny" restricts all operations, and "extend-cache" allows any cached ACLs to be used, ignoring their TTL - values. If a non-cached ACL is used, "extend-cache" acts like "deny". "async-cache" acts the same - way as "extend-cache" but performs updates asynchronously when ACL is present but its TTL is - expired. + values. If a non-cached ACL is used, "extend-cache" acts like "deny". + The value "async-cache" acts the same way as "extend-cache" but performs updates + asynchronously when ACL is present but its TTL is expired, thus, if latency is bad between + ACL authoritative and other datacenters, latency of operations is not impacted. * `acl_agent_master_token` - Used to access agent endpoints that require agent read From 684674b757b4f9fb50f2600513f5c05fc7950b55 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Mon, 2 Jul 2018 00:24:37 +0200 Subject: [PATCH 13/54] Trying to fix Travis tests --- GNUmakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GNUmakefile b/GNUmakefile index 5df83416b9..d510c0fe6a 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -158,7 +158,7 @@ test: other-consul dev-build vet @# hide it from travis as it exceeds their log limits and causes job to be @# terminated (over 4MB and over 10k lines in the UI). We need to output @# _something_ to stop them terminating us due to inactivity... - { go test $(GOTEST_FLAGS) -tags '$(GOTAGS)' -timeout 5m $(GOTEST_PKGS) 2>&1 ; echo $$? > exit-code ; } | tee test.log | egrep '^(ok|FAIL)\s*github.com/hashicorp/consul' + { go test $(GOTEST_FLAGS) -tags '$(GOTAGS)' -timeout 7m $(GOTEST_PKGS) 2>&1 ; echo $$? > exit-code ; } | tee test.log | egrep '^(ok|FAIL)\s*github.com/hashicorp/consul' @echo "Exit code: $$(cat exit-code)" >> test.log @# This prints all the race report between ====== lines @awk '/^WARNING: DATA RACE/ {do_print=1; print "=================="} do_print==1 {print} /^={10,}/ {do_print=0}' test.log || true From bd023f352e466b47a773241c34249ad822bc4c91 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Mon, 2 Jul 2018 17:39:34 +0200 Subject: [PATCH 14/54] Updated swith case to use same branch for async-cache and extend-cache --- agent/acl.go | 3 +-- agent/consul/acl.go | 3 +-- agent/consul/config.go | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/agent/acl.go b/agent/acl.go index 1143da97cc..5c4deb7cc8 100644 --- a/agent/acl.go +++ b/agent/acl.go @@ -103,8 +103,7 @@ func newACLManager(config *config.RuntimeConfig) (*aclManager, error) { down = acl.AllowAll() case "deny": down = acl.DenyAll() - case "extend-cache": - case "async-cache": + case "async-cache", "extend-cache": // Leave the down policy as nil to signal this. default: return nil, fmt.Errorf("invalid ACL down policy %q", config.ACLDownPolicy) diff --git a/agent/consul/acl.go b/agent/consul/acl.go index 23bdbff356..4dbfe2b39e 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -277,8 +277,7 @@ ACL_DOWN: case "allow": c.fireResult(id, acl.AllowAll(), nil) return - case "async-cache": - case "extend-cache": + case "async-cache", "extend-cache": if cached != nil { c.fireResult(id, cached.ACL, nil) return diff --git a/agent/consul/config.go b/agent/consul/config.go index 570a07c163..b878d9a99d 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -379,8 +379,7 @@ func (c *Config) CheckACL() error { switch c.ACLDownPolicy { case "allow": case "deny": - case "async-cache": - case "extend-cache": + case "async-cache", "extend-cache": default: return fmt.Errorf("Unsupported down ACL policy: %s", c.ACLDownPolicy) } From 1492243e0aaec34af642eb235fb424422c3dcd38 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 21 Jun 2018 15:42:28 -0700 Subject: [PATCH 15/54] connect/ca: add logic for pruning old stale RootCA entries --- agent/consul/connect_ca_endpoint.go | 2 + agent/consul/leader.go | 107 +++++++++++++++++++++++++++- agent/consul/leader_test.go | 63 ++++++++++++++++ agent/consul/server.go | 6 ++ agent/structs/connect_ca.go | 5 ++ 5 files changed, 182 insertions(+), 1 deletion(-) diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 47672ee559..1e04f6733e 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strings" + "time" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/connect" @@ -177,6 +178,7 @@ func (s *ConnectCA) ConfigurationSet( newRoot := *r if newRoot.Active { newRoot.Active = false + newRoot.RotateOutAt = time.Now().Add(caRootExpireDuration) } newRoots = append(newRoots, &newRoot) } diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 5fe4eebece..2b0497dc38 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -28,7 +28,18 @@ const ( barrierWriteTimeout = 2 * time.Minute ) -var minAutopilotVersion = version.Must(version.NewVersion("0.8.0")) +var ( + // caRootPruneInterval is how often we check for stale CARoots to remove. + caRootPruneInterval = time.Hour + + // caRootExpireDuration is the duration after which an inactive root is considered + // "expired". + caRootExpireDuration = 7 * 24 * time.Hour + + // minAutopilotVersion is the minimum Consul version in which Autopilot features + // are supported. + minAutopilotVersion = version.Must(version.NewVersion("0.8.0")) +) // monitorLeadership is used to monitor if we acquire or lose our role // as the leader in the Raft cluster. There is some work the leader is @@ -220,6 +231,8 @@ func (s *Server) establishLeadership() error { return err } + s.startCARootPruning() + s.setConsistentReadReady() return nil } @@ -236,6 +249,8 @@ func (s *Server) revokeLeadership() error { return err } + s.stopCARootPruning() + s.setCAProvider(nil, nil) s.resetConsistentReadReady() @@ -550,6 +565,96 @@ func (s *Server) setCAProvider(newProvider ca.Provider, root *structs.CARoot) { s.caProviderRoot = root } +// startCARootPruning starts a goroutine that looks for stale CARoots +// and removes them from the state store. +func (s *Server) startCARootPruning() { + if !s.config.ConnectEnabled { + return + } + + s.caPruningLock.Lock() + defer s.caPruningLock.Unlock() + + if s.caPruningEnabled { + return + } + + s.caPruningCh = make(chan struct{}) + + go func() { + ticker := time.NewTicker(caRootPruneInterval) + defer ticker.Stop() + + for { + select { + case <-s.caPruningCh: + return + case <-ticker.C: + if err := s.pruneCARoots(); err != nil { + s.logger.Printf("[ERR] connect: error pruning CA roots: %v", err) + } + } + } + }() + + s.caPruningEnabled = true +} + +// pruneCARoots looks for any CARoots that have been rotated out and expired. +func (s *Server) pruneCARoots() error { + idx, roots, err := s.fsm.State().CARoots(nil) + if err != nil { + return err + } + + var newRoots structs.CARoots + for _, r := range roots { + if !r.Active && !r.RotateOutAt.IsZero() && r.RotateOutAt.Before(time.Now()) { + s.logger.Printf("[INFO] connect: pruning old unused root CA (ID: %s)", r.ID) + continue + } + newRoot := *r + newRoots = append(newRoots, &newRoot) + } + + // Return early if there's nothing to remove. + if len(newRoots) == len(roots) { + return nil + } + + // Commit the new root state. + var args structs.CARequest + args.Op = structs.CAOpSetRoots + args.Index = idx + args.Roots = newRoots + resp, err := s.raftApply(structs.ConnectCARequestType, args) + if err != nil { + return err + } + if respErr, ok := resp.(error); ok { + return respErr + } + + return nil +} + +// stopCARootPruning stops the CARoot pruning process. +func (s *Server) stopCARootPruning() { + if !s.config.ConnectEnabled { + return + } + + s.caPruningLock.Lock() + defer s.caPruningLock.Unlock() + + if !s.caPruningEnabled { + return + } + + close(s.caPruningCh) + s.caPruningEnabled = false +} + // reconcileReaped is used to reconcile nodes that have failed and been reaped // from Serf but remain in the catalog. This is done by looking for unknown nodes with serfHealth checks registered. // We generate a "reap" event to cause the node to be cleaned up. diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go index da2092fdab..a685223cb3 100644 --- a/agent/consul/leader_test.go +++ b/agent/consul/leader_test.go @@ -5,12 +5,14 @@ import ( "testing" "time" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/testutil/retry" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/serf/serf" + "github.com/stretchr/testify/require" ) func TestLeader_RegisterMember(t *testing.T) { @@ -1001,3 +1003,64 @@ func TestLeader_ACL_Initialization(t *testing.T) { }) } } + +func TestLeader_CARootPruning(t *testing.T) { + t.Parallel() + + caRootExpireDuration = 500 * time.Millisecond + caRootPruneInterval = 200 * time.Millisecond + + require := require.New(t) + dir1, s1 := testServer(t) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + // Get the current root + rootReq := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var rootList structs.IndexedCARoots + require.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", rootReq, &rootList)) + require.Len(rootList.Roots, 1) + oldRoot := rootList.Roots[0] + + // Update the provider config to use a new private key, which should + // cause a rotation. + _, newKey, err := connect.GeneratePrivateKey() + require.NoError(err) + newConfig := &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "PrivateKey": newKey, + "RootCert": "", + "RotationPeriod": 90 * 24 * time.Hour, + }, + } + { + args := &structs.CARequest{ + Datacenter: "dc1", + Config: newConfig, + } + var reply interface{} + + require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) + } + + // Should have 2 roots now. + _, roots, err := s1.fsm.State().CARoots(nil) + require.NoError(err) + require.Len(roots, 2) + + time.Sleep(caRootExpireDuration * 2) + + // Now the old root should be pruned. + _, roots, err = s1.fsm.State().CARoots(nil) + require.NoError(err) + require.Len(roots, 1) + require.True(roots[0].Active) + require.NotEqual(roots[0].ID, oldRoot.ID) +} diff --git a/agent/consul/server.go b/agent/consul/server.go index ca363e7a1b..4520b11114 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -107,6 +107,12 @@ type Server struct { caProviderRoot *structs.CARoot caProviderLock sync.RWMutex + // caPruningCh is used to shut down the CA root pruning goroutine when we + // lose leadership. + caPruningCh chan struct{} + caPruningLock sync.RWMutex + caPruningEnabled bool + // Consul configuration config *Config diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 2577a99a69..52b9c16f15 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -73,6 +73,11 @@ type CARoot struct { // cannot be active. Active bool + // RotateOutAt is the time at which this CA can be removed from the state. + // This will only be set on roots that have been rotated out from being the + // active one. + RotateOutAt time.Time `json:"-"` + RaftIndex } From df8506e20043c38c55cb1fda275baf6a10a4cc50 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Mon, 2 Jul 2018 19:02:16 +0100 Subject: [PATCH 16/54] Move testing doubles to use data embedded in the HTML vs HTTP/fetch Previously `api-double` usage in ember would require a bunch of `fetch` requests to pull in the 'api double', this had a number of disadvantages. 1. The doubles needed to be available via HTTP, which meant a short term solution of rsyncing the double files over to `public` in order to be served over HTTP. An alternative to that would have been figuring out how to serve something straight from `node_modules`, which would have been preferable. 2. ember/testem would not serve dot files (so anything starting with a ., like `.config`. To solve this via ember/testem would have involved digging in to understand how to enable the serving of dot files. 3. ember/testem automatically rewrote urls for non-existant files to folders, i.e. adding a slash for you, so `/v1/connect/intentions` would be rewritten to `/v1/connect/intentions/`. This is undesirable, and solving this via ember/testem would have involved digging deep to disable that. Serving the files via HTTP has now changed. The double files are now embedded into the HTML has 'embedded templates' that can be found by using the url of the file and a simple `querySelector`. This of course only happens during testing and means I can fully control the 'serving' of the doubles now, so I can say goodbye to the need to move files around, worry about the need to serve dotfiles and the undesirable trailing slashes rewriting. Winner! Find the files and embedding them is done using a straightforward recursive-readdir-sync (the `content-for` functionality is a synchronous api) as oppose to getting stuck into `broccoli`. --- ui-v2/.ember-cli | 3 ++- ui-v2/GNUmakefile | 21 ++++++++++++--------- ui-v2/config/environment.js | 20 ++++++++++++-------- ui-v2/package.json | 13 ++++++------- ui-v2/tests/helpers/api.js | 12 +++++++++++- ui-v2/yarn.lock | 25 +++++++++++++++---------- 6 files changed, 58 insertions(+), 36 deletions(-) diff --git a/ui-v2/.ember-cli b/ui-v2/.ember-cli index ee64cfed2a..cdbed3326d 100644 --- a/ui-v2/.ember-cli +++ b/ui-v2/.ember-cli @@ -5,5 +5,6 @@ Setting `disableAnalytics` to true will prevent any data from being sent. */ - "disableAnalytics": false + "disableAnalytics": false, + "proxy": "http://localhost:3000" } diff --git a/ui-v2/GNUmakefile b/ui-v2/GNUmakefile index f703de4f4f..e87521b992 100644 --- a/ui-v2/GNUmakefile +++ b/ui-v2/GNUmakefile @@ -1,28 +1,31 @@ ROOT:=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST)))) all: build - + deps: node_modules - + build: deps yarn run build - + start: deps yarn run start - + +start-api: deps + yarn run start:api + test: deps yarn run test - + test-view: deps yarn run test:view - + lint: deps yarn run lint:js - + format: deps yarn run format:js - + node_modules: yarn.lock package.json yarn install - + .PHONY: all deps build start test test-view lint format diff --git a/ui-v2/config/environment.js b/ui-v2/config/environment.js index b72e7b00de..6ee649a70e 100644 --- a/ui-v2/config/environment.js +++ b/ui-v2/config/environment.js @@ -30,9 +30,9 @@ module.exports = function(environment) { ENV = Object.assign({}, ENV, { CONSUL_GIT_SHA: (function() { if (process.env.CONSUL_GIT_SHA) { - return process.env.CONSUL_GIT_SHA + return process.env.CONSUL_GIT_SHA; } - + return require('child_process') .execSync('git rev-parse --short HEAD') .toString() @@ -40,7 +40,7 @@ module.exports = function(environment) { })(), CONSUL_VERSION: (function() { if (process.env.CONSUL_VERSION) { - return process.env.CONSUL_VERSION + return process.env.CONSUL_VERSION; } // see /scripts/dist.sh:8 const version_go = `${path.dirname(path.dirname(__dirname))}/version/version.go`; @@ -53,13 +53,13 @@ module.exports = function(environment) { .trim() .split('"')[1]; })(), - CONSUL_BINARY_TYPE: (function() { + CONSUL_BINARY_TYPE: function() { if (process.env.CONSUL_BINARY_TYPE) { - return process.env.CONSUL_BINARY_TYPE + return process.env.CONSUL_BINARY_TYPE; } - - return "oss" - }), + + return 'oss'; + }, CONSUL_DOCUMENTATION_URL: 'https://www.consul.io/docs', CONSUL_COPYRIGHT_URL: 'https://www.hashicorp.com', CONSUL_COPYRIGHT_YEAR: '2018', @@ -86,6 +86,10 @@ module.exports = function(environment) { ENV.APP.rootElement = '#ember-testing'; ENV.APP.autoboot = false; + ENV['ember-cli-api-double'] = { + reader: 'html', + endpoints: ['/node_modules/@hashicorp/consul-api-double/v1'], + }; } if (environment === 'production') { diff --git a/ui-v2/package.json b/ui-v2/package.json index 1f89ef82bf..5dd0dee53b 100644 --- a/ui-v2/package.json +++ b/ui-v2/package.json @@ -1,6 +1,6 @@ { "name": "consul-ui", - "version": "2.0.0", + "version": "2.2.0", "private": true, "description": "The web ui for Consul, by HashiCorp.", "directories": { @@ -14,9 +14,9 @@ "lint:js": "eslint -c .eslintrc.js --fix ./*.js ./.*.js app config lib server tests", "format:js": "prettier --write \"{app,config,lib,server,tests}/**/*.js\" ./*.js ./.*.js", "start": "ember serve", - "test:sync": "rsync -aq ./node_modules/@hashicorp/consul-api-double/ ./public/consul-api-double/", - "test": "yarn run test:sync;ember test", - "test:view": "yarn run test:sync;ember test --server", + "start:api": "api-double --dir ./node_modules/@hashicorp/consul-api-double", + "test": "ember test", + "test:view": "ember test --server", "precommit": "lint-staged" }, "lint-staged": { @@ -29,10 +29,9 @@ "git add" ] }, - "dependencies": {}, "devDependencies": { - "@hashicorp/consul-api-double": "^1.0.0", - "@hashicorp/ember-cli-api-double": "^1.0.2", + "@hashicorp/consul-api-double": "^1.2.0", + "@hashicorp/ember-cli-api-double": "^1.3.0", "babel-plugin-transform-object-rest-spread": "^6.26.0", "base64-js": "^1.3.0", "broccoli-asset-rev": "^2.4.5", diff --git a/ui-v2/tests/helpers/api.js b/ui-v2/tests/helpers/api.js index 800672103e..75acba0027 100644 --- a/ui-v2/tests/helpers/api.js +++ b/ui-v2/tests/helpers/api.js @@ -1,4 +1,14 @@ import getAPI from '@hashicorp/ember-cli-api-double'; import setCookies from 'consul-ui/tests/helpers/set-cookies'; import typeToURL from 'consul-ui/tests/helpers/type-to-url'; -export default getAPI('/consul-api-double', setCookies, typeToURL); +import config from 'consul-ui/config/environment'; +const apiConfig = config['ember-cli-api-double']; +let path = '/consul-api-double'; +let reader; +if (apiConfig) { + const temp = apiConfig.endpoints[0].split('/'); + reader = apiConfig.reader; + temp.pop(); + path = temp.join('/'); +} +export default getAPI(path, setCookies, typeToURL, reader); diff --git a/ui-v2/yarn.lock b/ui-v2/yarn.lock index c604847c41..50303e742d 100644 --- a/ui-v2/yarn.lock +++ b/ui-v2/yarn.lock @@ -69,9 +69,9 @@ dependencies: "@glimmer/di" "^0.2.0" -"@hashicorp/api-double@^1.1.0": - version "1.2.0" - resolved "https://registry.yarnpkg.com/@hashicorp/api-double/-/api-double-1.2.0.tgz#d2846f79d086ac009673ae755da15301e0f2f7c3" +"@hashicorp/api-double@^1.3.0": + version "1.3.1" + resolved "https://registry.yarnpkg.com/@hashicorp/api-double/-/api-double-1.3.1.tgz#fd9d706674b934857a638459c2bb52d2f2809455" dependencies: "@gardenhq/o" "^8.0.1" "@gardenhq/tick-control" "^2.0.0" @@ -81,20 +81,21 @@ faker "^4.1.0" js-yaml "^3.10.0" -"@hashicorp/consul-api-double@^1.0.0": - version "1.1.0" - resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-1.1.0.tgz#658f9e89208fa23f251ca66c66aeb7241a13f23f" - -"@hashicorp/ember-cli-api-double@^1.0.2": +"@hashicorp/consul-api-double@^1.2.0": version "1.2.0" - resolved "https://registry.yarnpkg.com/@hashicorp/ember-cli-api-double/-/ember-cli-api-double-1.2.0.tgz#aed3a9659abb3f3c56d77e400abc7fcbdcf2b78b" + resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-1.2.0.tgz#2cd2a991818e13e7b97803af3d62ec6c9cb83b28" + +"@hashicorp/ember-cli-api-double@^1.3.0": + version "1.3.0" + resolved "https://registry.yarnpkg.com/@hashicorp/ember-cli-api-double/-/ember-cli-api-double-1.3.0.tgz#d07b5b11701cd55d6b01cb8a47ce17c4bac21fed" dependencies: - "@hashicorp/api-double" "^1.1.0" + "@hashicorp/api-double" "^1.3.0" array-range "^1.0.1" ember-cli-babel "^6.6.0" js-yaml "^3.11.0" merge-options "^1.0.1" pretender "^2.0.0" + recursive-readdir-sync "^1.0.6" "@sinonjs/formatio@^2.0.0": version "2.0.0" @@ -7735,6 +7736,10 @@ recast@^0.11.17, recast@^0.11.3: private "~0.1.5" source-map "~0.5.0" +recursive-readdir-sync@^1.0.6: + version "1.0.6" + resolved "https://registry.yarnpkg.com/recursive-readdir-sync/-/recursive-readdir-sync-1.0.6.tgz#1dbf6d32f3c5bb8d3cde97a6c588d547a9e13d56" + redent@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/redent/-/redent-1.0.0.tgz#cf916ab1fd5f1f16dfb20822dd6ec7f730c2afde" From 77fe08b7c9bdb81b842c66c3baa1aafe5e6d2bff Mon Sep 17 00:00:00 2001 From: Siva Date: Thu, 28 Jun 2018 22:09:15 -0400 Subject: [PATCH 17/54] Website: Added more telemetry metrics --- website/source/docs/agent/telemetry.html.md | 92 +++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/website/source/docs/agent/telemetry.html.md b/website/source/docs/agent/telemetry.html.md index a0a9685c36..b7356cc179 100644 --- a/website/source/docs/agent/telemetry.html.md +++ b/website/source/docs/agent/telemetry.html.md @@ -355,6 +355,62 @@ These metrics are used to monitor the health of the Consul servers. Unit Type + + `consul.raft.fsm.snapshot` + This metric measures the time taken by the FSM to record the current state for the snapshot. + ms + timer + + + + `consul.raft.fsm.apply` + This metric gives the number of logs committed since the last interval. + commit logs / interval + counter + + + + `consul.raft.fsm.restore` + This metric measures the time taken by the FSM to restore its state from a snapshot. + ms + timer + + + `consul.raft.snapshot.create` + This metric measures the time taken to initialize the snapshot process. + ms + timer + + + `consul.raft.snapshot.persist` + This metric measures the time taken to dump the current snapshot taken by the Consul agent to the disk. + ms + timer + + + `consul.raft.snapshot.takeSnapshot` + This metric measures the total time involved in taking the current snapshot (creating one and persisting it) by the Consul agent. + ms + timer + + + `consul.raft.replication.heartbeat` + This metric measures the time taken to invoke appendEntries on a peer, so that it doesn’t timeout on a periodic basis. + ms + timer + + + `consul.serf.snapshot.appendLine` + This metric measures the time taken by the Consul agent to append an entry into the existing log. + ms + timer + + + `consul.serf.snapshot.compact` + This metric measures the time by the Consul agent to compact a log. This operation occurs only when the snapshot becomes too large enough to justify the compaction . + ms + timer + `consul.raft.state.leader` This increments whenever a Consul server becomes a leader. If there are frequent leadership changes this may be indication that the servers are overloaded and aren't meeting the soft real-time requirements for Raft, or that there are networking problems between the servers. @@ -655,6 +711,42 @@ These metrics give insight into the health of the cluster as a whole. suspect messages received / interval counter + + `consul.memberlist.gossip` + This metric gives the number of gossips (messages) broadcasted to a set of randomly selected nodes. + messages / Interval + counter + + + `consul.memberlist.msg_alive` + This metric counts the number of alive agents, that the agent has mapped out so far, based on the message information given by the network layer. + nodes / Interval + counter + + + `consul.memberlist.msg_dead` + This metric gives the number of dead agents, that the agent has mapped out so far, based on the message information given by the network layer. + nodes / Interval + counter + + + `consul.memberlist.msg_suspect` + This metric gives the number of suspect nodes, that the agent has mapped out so far, based on the message information given by the network layer. + nodes / Interval + counter + + + `consul.memberlist.probeNode` + This metric measures the time taken to perform a single round of failure detection on a select agent. + nodes / Interval + counter + + + `consul.memberlist.pushPullNode` + This metric measures the number of agents that have exchanged state with this agent. + nodes / Interval + counter + `consul.serf.member.flap` Available in Consul 0.7 and later, this increments when an agent is marked dead and then recovers within a short time period. This can be an indicator of overloaded agents, network problems, or configuration errors where agents can not connect to each other on the [required ports](/docs/agent/options.html#ports). From a47a0b617f7fc174bf2682c02d6a18d875116c23 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 3 Jul 2018 13:23:45 +0100 Subject: [PATCH 18/54] Hedge for when consul sends nodes with an empty ID --- ui-v2/app/adapters/node.js | 3 ++ .../acceptance/dc/nodes/empty-ids.feature | 33 +++++++++++++++++++ .../steps/dc/nodes/empty-ids-steps.js | 10 ++++++ 3 files changed, 46 insertions(+) create mode 100644 ui-v2/tests/acceptance/dc/nodes/empty-ids.feature create mode 100644 ui-v2/tests/acceptance/steps/dc/nodes/empty-ids-steps.js diff --git a/ui-v2/app/adapters/node.js b/ui-v2/app/adapters/node.js index 0890a8aa1a..22460875ad 100644 --- a/ui-v2/app/adapters/node.js +++ b/ui-v2/app/adapters/node.js @@ -23,6 +23,9 @@ export default Adapter.extend({ break; default: response = response.map((item, i, arr) => { + if (item[SLUG_KEY] === '') { + item[SLUG_KEY] = item['Node']; + } return { ...item, ...{ diff --git a/ui-v2/tests/acceptance/dc/nodes/empty-ids.feature b/ui-v2/tests/acceptance/dc/nodes/empty-ids.feature new file mode 100644 index 0000000000..434766775f --- /dev/null +++ b/ui-v2/tests/acceptance/dc/nodes/empty-ids.feature @@ -0,0 +1,33 @@ +@setupApplicationTest +Feature: Hedge for if nodes come in over the API with no ID + Scenario: A node list with some missing IDs + Given 1 datacenter model with the value "dc-1" + And 5 node models from yaml + --- + - ID: id-1 + Node: name-1 + - ID: "" + Node: name-2 + - ID: "" + Node: name-3 + - ID: "" + Node: name-4 + - ID: "" + Node: name-5 + --- + When I visit the nodes page for yaml + --- + dc: dc-1 + --- + Then the url should be /dc-1/nodes + Then I see name on the nodes like yaml + --- + - name-1 + - name-2 + - name-3 + - name-4 + - name-5 + +@ignore + Scenario: Visually comparing + Then the ".unhealthy" element should look like the "/node_modules/@hashicorp/consul-testing-extras/fixtures/dc/nodes/empty-ids.png" image diff --git a/ui-v2/tests/acceptance/steps/dc/nodes/empty-ids-steps.js b/ui-v2/tests/acceptance/steps/dc/nodes/empty-ids-steps.js new file mode 100644 index 0000000000..a7eff3228b --- /dev/null +++ b/ui-v2/tests/acceptance/steps/dc/nodes/empty-ids-steps.js @@ -0,0 +1,10 @@ +import steps from '../../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} From 6398fb02c78bb87a01c16da139a217e58b344e8e Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 3 Jul 2018 14:48:04 +0100 Subject: [PATCH 19/54] Ensure we catch empty ID's for single nodes also I don't think this would have a large effect on the UI whichever but best to make sure --- ui-v2/app/adapters/node.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ui-v2/app/adapters/node.js b/ui-v2/app/adapters/node.js index 22460875ad..902687ff40 100644 --- a/ui-v2/app/adapters/node.js +++ b/ui-v2/app/adapters/node.js @@ -1,6 +1,14 @@ import Adapter from './application'; import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/node'; import { OK as HTTP_OK } from 'consul-ui/utils/http/status'; +// TODO: Looks like ID just isn't used at all +// consider just using .Node for the SLUG_KEY +const fillSlug = function(item) { + if (item[SLUG_KEY] === '') { + item[SLUG_KEY] = item['Node']; + } + return item; +}; export default Adapter.extend({ urlForQuery: function(query, modelName) { return this.appendURL('internal/ui/nodes', [], this.cleanQuery(query)); @@ -14,6 +22,7 @@ export default Adapter.extend({ const url = this.parseURL(requestData.url); switch (true) { case this.isQueryRecord(url): + response = fillSlug(response); response = { ...response, ...{ @@ -23,9 +32,7 @@ export default Adapter.extend({ break; default: response = response.map((item, i, arr) => { - if (item[SLUG_KEY] === '') { - item[SLUG_KEY] = item['Node']; - } + item = fillSlug(item); return { ...item, ...{ From 7f35ca33f4a2753f6d6e7a7b635e1594f977c38d Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Tue, 3 Jul 2018 14:54:44 +0100 Subject: [PATCH 20/54] Remove the TODO latency measurement. --- website/source/docs/connect/proxies.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/connect/proxies.html.md b/website/source/docs/connect/proxies.html.md index 32fbe2603a..2d60c92f45 100644 --- a/website/source/docs/connect/proxies.html.md +++ b/website/source/docs/connect/proxies.html.md @@ -41,7 +41,7 @@ The default managed proxy is a basic proxy built-in to Consul and written in Go. Having a basic built-in proxy allows Consul to have a sane default with performance that is good enough for most workloads. In some basic benchmarks, the service-to-service communication over the built-in proxy -could sustain 5 Gbps with a per-hop latency of less than X microseconds. Therefore, +could sustain 5 Gbps with sub-millisecond latency. Therefore, the performance impact of even the basic built-in proxy is minimal. Consul will be From 9579ba4e12bf8a0d5366812117252f2e405f05b9 Mon Sep 17 00:00:00 2001 From: Siva Date: Tue, 3 Jul 2018 10:27:01 -0400 Subject: [PATCH 21/54] Website: Added more telemetry details for raft and memberlist. --- website/source/docs/agent/telemetry.html.md | 43 +++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/website/source/docs/agent/telemetry.html.md b/website/source/docs/agent/telemetry.html.md index b7356cc179..df9b843e10 100644 --- a/website/source/docs/agent/telemetry.html.md +++ b/website/source/docs/agent/telemetry.html.md @@ -429,6 +429,25 @@ These metrics are used to monitor the health of the Consul servers. raft transactions / interval counter + + `consul.raft.barrier` + This metric counts the number of times the agent has started the barrier i.e the number of times it has + issued a blocking call, to ensure that the agent has all the pending operations that were queued, to be applied to the agent's FSM. + blocks / interval + counter + + + `consul.raft.verify_leader` + This metric counts the number of times an agent checks whether it is still the leader or not + checks / interval + Counter + + + `consul.raft.restore` + This metric counts the number of times the restore operation has been performed by the agent. Here, restore refers to the action of raft consuming an external snapshot to restore its state. + operation invoked / interval + counter + `consul.raft.commitTime` This measures the time it takes to commit a new entry to the Raft log on the leader. @@ -705,6 +724,30 @@ These metrics give insight into the health of the cluster as a whole. Unit Type + + `consul.memberlist.degraded.probe` + This metric counts the number of times the agent has performed failure detection on an other agent at a slower probe rate. The agent uses its own health metric as an indicator to perform this action. (If its health score is low, means that the node is healthy, and vice versa.) + probes / interval + counter + + + `consul.memberlist.degraded.timeout` + This metric counts the number of times an agent was marked as a dead node, whilst not getting enough confirmations from a randomly selected list of agent nodes in an agent's membership. + occurrence / interval + counter + + + `consul.memberlist.msg.dead` + This metric counts the number of times an agent has marked another agent to be a dead node. + messages / interval + counter + + + `consul.memberlist.health.score` + This metric emits the agent's updated health score. This score is updated whenever the agent notices any changes in the response, from a set of randomly probed agents. This value ranges from 0-8, the lowest indicating the agent is healthy and vice versa. + score + gauge + `consul.memberlist.msg.suspect` This increments when an agent suspects another as failed when executing random probes as part of the gossip protocol. These can be an indicator of overloaded agents, network problems, or configuration errors where agents can not connect to each other on the [required ports](/docs/agent/options.html#ports). From b445df39bbce32528fa065021fc119dd7f74f869 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 3 Jul 2018 15:40:15 +0100 Subject: [PATCH 22/54] Use html5 oninput instead of onkeyup for native textual inputs This enables people to enter things using the mouse to paste for example, plus possible other things. As an aside it also answers my query regarding `fillIn` for testing, nothing needs to be actually _typed_ anymore! Doh --- .../templates/components/freetext-filter.hbs | 2 +- ui-v2/app/templates/dc/kv/-form.hbs | 6 +++--- .../acceptance/components/acl-filter.feature | 4 ++-- .../components/catalog-filter.feature | 4 ++-- .../acceptance/components/kv-filter.feature | 2 +- .../acceptance/components/text-input.feature | 21 +++++++++++++++++++ ui-v2/tests/acceptance/dc/acls/update.feature | 2 +- ui-v2/tests/acceptance/dc/kvs/update.feature | 2 +- .../steps/components/text-input-steps.js | 10 +++++++++ ui-v2/tests/acceptance/token-header.feature | 2 +- ui-v2/tests/pages/dc/acls/edit.js | 3 ++- ui-v2/tests/pages/dc/kv/edit.js | 3 ++- ui-v2/tests/steps.js | 2 +- 13 files changed, 48 insertions(+), 15 deletions(-) create mode 100644 ui-v2/tests/acceptance/components/text-input.feature create mode 100644 ui-v2/tests/acceptance/steps/components/text-input-steps.js diff --git a/ui-v2/app/templates/components/freetext-filter.hbs b/ui-v2/app/templates/components/freetext-filter.hbs index 590726e08c..417a3787d2 100644 --- a/ui-v2/app/templates/components/freetext-filter.hbs +++ b/ui-v2/app/templates/components/freetext-filter.hbs @@ -1,6 +1,6 @@ {{!
}} {{!
}} \ No newline at end of file diff --git a/ui-v2/app/templates/dc/kv/-form.hbs b/ui-v2/app/templates/dc/kv/-form.hbs index 6303969045..9b859c5a19 100644 --- a/ui-v2/app/templates/dc/kv/-form.hbs +++ b/ui-v2/app/templates/dc/kv/-form.hbs @@ -3,14 +3,14 @@ {{#if create }} {{/if}} {{#if (or (eq (left-trim item.Key parent.Key) '') (not-eq (last item.Key) '/')) }}
diff --git a/ui-v2/tests/acceptance/components/acl-filter.feature b/ui-v2/tests/acceptance/components/acl-filter.feature index 0d88655225..15ffad0a2c 100644 --- a/ui-v2/tests/acceptance/components/acl-filter.feature +++ b/ui-v2/tests/acceptance/components/acl-filter.feature @@ -25,12 +25,12 @@ Feature: dc / components /acl filter: Acl Filter When I click all on the filter Then I see allIsSelected on the filter - Then I type with yaml + Then I fill in with yaml --- s: Anonymous Token --- And I see 1 [Model] model with the name "Anonymous Token" - Then I type with yaml + Then I fill in with yaml --- s: secret --- diff --git a/ui-v2/tests/acceptance/components/catalog-filter.feature b/ui-v2/tests/acceptance/components/catalog-filter.feature index 2635228002..9db4e0ee7c 100644 --- a/ui-v2/tests/acceptance/components/catalog-filter.feature +++ b/ui-v2/tests/acceptance/components/catalog-filter.feature @@ -47,7 +47,7 @@ Feature: components / catalog-filter When I click all on the filter And I see allIsSelected on the filter - Then I type with yaml + Then I fill in with yaml --- s: [Model]-0 --- @@ -75,7 +75,7 @@ Feature: components / catalog-filter When I click services on the tabs And I see servicesIsSelected on the tabs - Then I type with yaml + Then I fill in with yaml --- s: 65535 --- diff --git a/ui-v2/tests/acceptance/components/kv-filter.feature b/ui-v2/tests/acceptance/components/kv-filter.feature index ebe740c466..f2cedbddbd 100644 --- a/ui-v2/tests/acceptance/components/kv-filter.feature +++ b/ui-v2/tests/acceptance/components/kv-filter.feature @@ -12,7 +12,7 @@ Feature: components / kv-filter dc: dc-1 --- Then the url should be [Url] - Then I type with yaml + Then I fill in with yaml --- s: [Text] --- diff --git a/ui-v2/tests/acceptance/components/text-input.feature b/ui-v2/tests/acceptance/components/text-input.feature new file mode 100644 index 0000000000..02bc044138 --- /dev/null +++ b/ui-v2/tests/acceptance/components/text-input.feature @@ -0,0 +1,21 @@ +@setupApplicationTest +Feature: Text input + Background: + Given 1 datacenter model with the value "dc-1" + Scenario: + When I visit the [Page] page for yaml + --- + dc: dc-1 + --- + Then the url should be [Url] + Then I fill in with json + --- + [Data] + --- + Then I see submitIsEnabled + Where: + -------------------------------------------------------------------------------- + | Page | Url | Data | + | kv | /dc-1/kv/create | {"additional": "hi", "value": "there"} | + | acl | /dc-1/acls/create | {"name": "hi"} | + -------------------------------------------------------------------------------- diff --git a/ui-v2/tests/acceptance/dc/acls/update.feature b/ui-v2/tests/acceptance/dc/acls/update.feature index 77c6417fd7..f79d637c70 100644 --- a/ui-v2/tests/acceptance/dc/acls/update.feature +++ b/ui-v2/tests/acceptance/dc/acls/update.feature @@ -12,7 +12,7 @@ Feature: dc / acls / update: ACL Update acl: key --- Then the url should be /datacenter/acls/key - Then I type with yaml + Then I fill in with yaml --- name: [Name] --- diff --git a/ui-v2/tests/acceptance/dc/kvs/update.feature b/ui-v2/tests/acceptance/dc/kvs/update.feature index b6a880c8dc..1f00f7c257 100644 --- a/ui-v2/tests/acceptance/dc/kvs/update.feature +++ b/ui-v2/tests/acceptance/dc/kvs/update.feature @@ -12,7 +12,7 @@ Feature: dc / kvs / update: KV Update kv: [Name] --- Then the url should be /datacenter/kv/[Name]/edit - Then I type with yaml + Then I fill in with yaml --- value: [Value] --- diff --git a/ui-v2/tests/acceptance/steps/components/text-input-steps.js b/ui-v2/tests/acceptance/steps/components/text-input-steps.js new file mode 100644 index 0000000000..960cdf533d --- /dev/null +++ b/ui-v2/tests/acceptance/steps/components/text-input-steps.js @@ -0,0 +1,10 @@ +import steps from '../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} diff --git a/ui-v2/tests/acceptance/token-header.feature b/ui-v2/tests/acceptance/token-header.feature index 750a3fe745..6ca14431c8 100644 --- a/ui-v2/tests/acceptance/token-header.feature +++ b/ui-v2/tests/acceptance/token-header.feature @@ -16,7 +16,7 @@ Feature: token headers Given 1 datacenter model with the value "datacenter" When I visit the settings page Then the url should be /settings - Then I type with yaml + Then I fill in with yaml --- token: [Token] --- diff --git a/ui-v2/tests/pages/dc/acls/edit.js b/ui-v2/tests/pages/dc/acls/edit.js index d15301ef46..7056072201 100644 --- a/ui-v2/tests/pages/dc/acls/edit.js +++ b/ui-v2/tests/pages/dc/acls/edit.js @@ -1,4 +1,4 @@ -import { create, clickable, triggerable } from 'ember-cli-page-object'; +import { create, clickable, triggerable, is } from 'ember-cli-page-object'; import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; export default create({ @@ -7,4 +7,5 @@ export default create({ // fillIn: fillable('input, textarea, [contenteditable]'), name: triggerable('keypress', '[name="name"]'), submit: clickable('[type=submit]'), + submitIsEnabled: is(':not(:disabled)', '[type=submit]'), }); diff --git a/ui-v2/tests/pages/dc/kv/edit.js b/ui-v2/tests/pages/dc/kv/edit.js index c8af7bae0f..df07d0df5b 100644 --- a/ui-v2/tests/pages/dc/kv/edit.js +++ b/ui-v2/tests/pages/dc/kv/edit.js @@ -1,4 +1,4 @@ -import { create, clickable } from 'ember-cli-page-object'; +import { create, clickable, is } from 'ember-cli-page-object'; import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; export default create({ @@ -7,4 +7,5 @@ export default create({ // fillIn: fillable('input, textarea, [contenteditable]'), // name: triggerable('keypress', '[name="additional"]'), submit: clickable('[type=submit]'), + submitIsEnabled: is(':not(:disabled)', '[type=submit]'), }); diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index 383dc4bf7c..901843bd12 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -335,7 +335,7 @@ export default function(assert) { `Expected to not see ${property} on ${component}` ); }) - .then(['I see $property'], function(property, component) { + .then(['I see $property'], function(property) { assert.ok(currentPage[property], `Expected to see ${property}`); }) .then(['I see the text "$text" in "$selector"'], function(text, selector) { From 5d8bf053e0742f623290dabea13bdc8933db6c7f Mon Sep 17 00:00:00 2001 From: Siva Date: Tue, 3 Jul 2018 10:59:31 -0400 Subject: [PATCH 23/54] Website/Telemetry: Errata for snapshot.compact and reworded memberlist.health.score --- website/source/docs/agent/telemetry.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/agent/telemetry.html.md b/website/source/docs/agent/telemetry.html.md index df9b843e10..4028ed9d10 100644 --- a/website/source/docs/agent/telemetry.html.md +++ b/website/source/docs/agent/telemetry.html.md @@ -407,7 +407,7 @@ These metrics are used to monitor the health of the Consul servers. `consul.serf.snapshot.compact` - This metric measures the time by the Consul agent to compact a log. This operation occurs only when the snapshot becomes too large enough to justify the compaction . + This metric measures the time taken by the Consul agent to compact a log. This operation occurs only when the snapshot becomes large enough to justify the compaction . ms timer @@ -744,7 +744,7 @@ These metrics give insight into the health of the cluster as a whole. `consul.memberlist.health.score` - This metric emits the agent's updated health score. This score is updated whenever the agent notices any changes in the response, from a set of randomly probed agents. This value ranges from 0-8, the lowest indicating the agent is healthy and vice versa. + This metric describes a node's perception of its own health based on how well it is meeting the soft real-time requirements of the protocol. This metric ranges from 0 to 8, where 0 indicates "totally healthy". This health score is used to scale the time between outgoing probes, and higher scores translate into longer probing intervals. For more details see section IV of the Lifeguard paper: https://arxiv.org/pdf/1707.00788.pdf score gauge From a1a62e3b914c0a0a06ceee9281247ebe273bb2ef Mon Sep 17 00:00:00 2001 From: Siva Date: Tue, 3 Jul 2018 17:37:29 -0400 Subject: [PATCH 24/54] Website/Docs/Telemetry: Added more raft and memberlist items. --- website/source/docs/agent/telemetry.html.md | 90 +++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/website/source/docs/agent/telemetry.html.md b/website/source/docs/agent/telemetry.html.md index 4028ed9d10..f78ba9118b 100644 --- a/website/source/docs/agent/telemetry.html.md +++ b/website/source/docs/agent/telemetry.html.md @@ -466,6 +466,72 @@ These metrics are used to monitor the health of the Consul servers. ms timer + + `consul.raft.state.follower` + This metric counts the number of times an agent has entered the follower mode. This happens when a new agent joins the cluster or after the end of a leader election. + follower state entered / interval + counter + + + `consul.raft.transistion.heartbeat_timeout` + This metric gives the number of times an agent has transistioned to the Candidate state, after receive no heartbeat messages from the last known leader. + timeouts / interval + counter + + + `consul.raft.restoreUserSnapshot` + This metric measures the time taken by the agent to restore the FSM state from a user's snapshot + ms + timer + + + `consul.raft.rpc.processHeartBeat` + This metric measures the time taken to process a heartbeat request. + ms + timer + + + `consul.raft.rpc.appendEntries` + This metric measures the time taken to process an append entries RPC call from an agent. + ms + timer + + + `consul.raft.rpc.appendEntries.storeLogs` + This metric measures the time taken to add any outstanding logs for an agent, since the last appendEntries was invoked + ms + timer + + + `consul.raft.rpc.appendEntries.processLogs` + This metric measures the time taken to process the outstanding log entries of an agent. + ms + timer + + + + `consul.raft.rpc.requestVote` + This metric measures the time taken to process the request vote RPC call. + ms + timer + + + `consul.raft.rpc.installSnapshot` + This metric measures the time taken to process the installSnapshot RPC call. This metric should only be seen on agents which are currently in the follower state. + ms + timer + + `consul.raft.replication.appendEntries.rpc` + This metric measures the time taken by the append entries RFC, to replicate the log entries of a leader agent onto its follower agent(s) + ms + timer + + + `consul.raft.replication.appendEntries.logs` + This metric measures the number of logs replicated to an agent, to bring it upto speed with the leader's logs. + logs appended/ interval + counter + `consul.raft.leader.lastContact` This will only be emitted by the Raft leader and measures the time since the leader was last able to contact the follower nodes when checking its leader lease. It can be used as a measure for how stable the Raft timing is and how close the leader is to timing out its lease.

The lease timeout is 500 ms times the [`raft_multiplier` configuration](/docs/agent/options.html#raft_multiplier), so this telemetry value should not be getting close to that configured value, otherwise the Raft timing is marginal and might need to be tuned, or more powerful servers might be needed. See the [Server Performance](/docs/guides/performance.html) guide for more details. @@ -754,6 +820,30 @@ These metrics give insight into the health of the cluster as a whole. suspect messages received / interval counter + + `consul.memberlist.tcp.accept` + This metric counts the number of times an agent has accepted an incoming TCP stream connection. + connections accepted / interval + counter + + + `consul.memberlist.udp.sent/received` + This metric measures the total number of bytes sent/received by an agent through the UDP protocol. + bytes sent or bytes received / interval + counter + + + `consul.memberlist.tcp.connect` + This metric counts the number of times an agent has initiated a push/pull sync with an other agent. + push/pull initiated / interval + counter + + + `consul.memberlist.tcp.sent` + This metric measures the total number of bytes sent by an agent through the TCP protocol + bytes sent / interval + counter + `consul.memberlist.gossip` This metric gives the number of gossips (messages) broadcasted to a set of randomly selected nodes. From bc9c5927b78d896341f254e6b3548f7ca0c19f92 Mon Sep 17 00:00:00 2001 From: Siva Prasad Date: Tue, 3 Jul 2018 18:18:57 -0400 Subject: [PATCH 25/54] Website/Docs/Telemetry : Errata --- website/source/docs/agent/telemetry.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/agent/telemetry.html.md b/website/source/docs/agent/telemetry.html.md index f78ba9118b..fc93d61fa1 100644 --- a/website/source/docs/agent/telemetry.html.md +++ b/website/source/docs/agent/telemetry.html.md @@ -474,7 +474,7 @@ These metrics are used to monitor the health of the Consul servers. `consul.raft.transistion.heartbeat_timeout` - This metric gives the number of times an agent has transistioned to the Candidate state, after receive no heartbeat messages from the last known leader. + This metric gives the number of times an agent has transitioned to the Candidate state, after receive no heartbeat messages from the last known leader. timeouts / interval counter From ed286585e92d9813f451a6999623d8fa75b2ded9 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 13:21:30 +0100 Subject: [PATCH 26/54] Add some low hanging intention tests, basically add intentions to others 1. There are various things tests that can just have intentions added into them, like filters and such like, add intentions to these 2. Start thinking about being able to negate steps easily, which will lead on to a cleanup of the steps --- ui-v2/app/templates/dc/intentions/index.hbs | 6 +- .../acceptance/components/acl-filter.feature | 2 +- .../components/intention-filter.feature | 55 +++++++++++++++++++ .../acceptance/components/text-input.feature | 2 +- .../tests/acceptance/page-navigation.feature | 32 ++++++----- .../components/intention-filter-steps.js | 10 ++++ ui-v2/tests/acceptance/submit-blank.feature | 11 ++-- ui-v2/tests/pages.js | 2 + .../pages/components/intention-filter.js | 9 +++ ui-v2/tests/pages/components/page.js | 2 +- ui-v2/tests/pages/dc/intentions/index.js | 16 ++++++ ui-v2/tests/steps.js | 6 +- 12 files changed, 126 insertions(+), 27 deletions(-) create mode 100644 ui-v2/tests/acceptance/components/intention-filter.feature create mode 100644 ui-v2/tests/acceptance/steps/components/intention-filter-steps.js create mode 100644 ui-v2/tests/pages/components/intention-filter.js create mode 100644 ui-v2/tests/pages/dc/intentions/index.js diff --git a/ui-v2/app/templates/dc/intentions/index.hbs b/ui-v2/app/templates/dc/intentions/index.hbs index 60ed96a2fe..946524d05f 100644 --- a/ui-v2/app/templates/dc/intentions/index.hbs +++ b/ui-v2/app/templates/dc/intentions/index.hbs @@ -27,7 +27,7 @@ {{/block-slot}} {{#block-slot 'row'}} - + {{#if (eq item.SourceName '*') }} All Services (*) {{else}} @@ -35,10 +35,10 @@ {{/if}} - + {{item.Action}} - + {{#if (eq item.DestinationName '*') }} All Services (*) {{else}} diff --git a/ui-v2/tests/acceptance/components/acl-filter.feature b/ui-v2/tests/acceptance/components/acl-filter.feature index 15ffad0a2c..6d90a68f78 100644 --- a/ui-v2/tests/acceptance/components/acl-filter.feature +++ b/ui-v2/tests/acceptance/components/acl-filter.feature @@ -1,5 +1,5 @@ @setupApplicationTest -Feature: dc / components /acl filter: Acl Filter +Feature: components / acl filter: Acl Filter In order to find the acl token I'm looking for easier As a user I should be able to filter by type and freetext search tokens by name and token diff --git a/ui-v2/tests/acceptance/components/intention-filter.feature b/ui-v2/tests/acceptance/components/intention-filter.feature new file mode 100644 index 0000000000..c797dee40f --- /dev/null +++ b/ui-v2/tests/acceptance/components/intention-filter.feature @@ -0,0 +1,55 @@ +@setupApplicationTest +Feature: components / intention filter: Intention Filter + In order to find the intention I'm looking for easier + As a user + I should be able to filter by 'policy' (allow/deny) and freetext search tokens by source and destination + Scenario: Filtering [Model] + Given 1 datacenter model with the value "dc-1" + And 2 [Model] models + When I visit the [Page] page for yaml + --- + dc: dc-1 + --- + Then the url should be [Url] + + Then I see 2 [Model] models + And I see allIsSelected on the filter + + When I click allow on the filter + Then I see allowIsSelected on the filter + And I see 1 [Model] model + And I see 1 [Model] model with the action "allow" + + When I click deny on the filter + Then I see denyIsSelected on the filter + And I see 1 [Model] model + And I see 1 [Model] model with the action "deny" + + When I click all on the filter + Then I see 2 [Model] models + Then I see allIsSelected on the filter + Then I fill in with yaml + --- + s: alarm + --- + And I see 1 [Model] model + And I see 1 [Model] model with the source "alarm" + Then I fill in with yaml + --- + s: feed + --- + And I see 1 [Model] model + And I see 1 [Model] model with the destination "feed" + Then I fill in with yaml + --- + s: transmitter + --- + And I see 2 [Model] models + And I see 1 [Model] model with the source "transmitter" + And I see 1 [Model] model with the destination "transmitter" + + Where: + --------------------------------------------- + | Model | Page | Url | + | intention | intentions | /dc-1/intentions | + --------------------------------------------- diff --git a/ui-v2/tests/acceptance/components/text-input.feature b/ui-v2/tests/acceptance/components/text-input.feature index 02bc044138..e8d531b221 100644 --- a/ui-v2/tests/acceptance/components/text-input.feature +++ b/ui-v2/tests/acceptance/components/text-input.feature @@ -1,5 +1,5 @@ @setupApplicationTest -Feature: Text input +Feature: components / text-input: Text input Background: Given 1 datacenter model with the value "dc-1" Scenario: diff --git a/ui-v2/tests/acceptance/page-navigation.feature b/ui-v2/tests/acceptance/page-navigation.feature index 360b910757..73d83beb18 100644 --- a/ui-v2/tests/acceptance/page-navigation.feature +++ b/ui-v2/tests/acceptance/page-navigation.feature @@ -16,13 +16,14 @@ Feature: Page Navigation When I click [Link] on the navigation Then the url should be [Url] Where: - -------------------------------------- - | Link | Url | - | nodes | /dc-1/nodes | - | kvs | /dc-1/kv | - | acls | /dc-1/acls | - | settings | /settings | - -------------------------------------- + ---------------------------------------- + | Link | Url | + | nodes | /dc-1/nodes | + | kvs | /dc-1/kv | + | acls | /dc-1/acls | + | intentions | /dc-1/intentions | + | settings | /settings | + ---------------------------------------- Scenario: Clicking a [Item] in the [Model] listing When I visit the [Model] page for yaml --- @@ -31,13 +32,14 @@ Feature: Page Navigation When I click [Item] on the [Model] Then the url should be [Url] Where: - -------------------------------------------------------- - | Item | Model | Url | - | service | services | /dc-1/services/service-0 | - | node | nodes | /dc-1/nodes/node-0 | - | kv | kvs | /dc-1/kv/necessitatibus-0/edit | - | acl | acls | /dc-1/acls/anonymous | - -------------------------------------------------------- + ------------------------------------------------------------------------------------- + | Item | Model | Url | + | service | services | /dc-1/services/service-0 | + | node | nodes | /dc-1/nodes/node-0 | + | kv | kvs | /dc-1/kv/necessitatibus-0/edit | + | intention | intentions | /dc-1/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca | + | acl | acls | /dc-1/acls/anonymous | + ------------------------------------------------------------------------------------- @ignore - Scenario: Clicking a kv in the kvs listing, without depending on the salt ^ + Scenario: Clicking items in the listings, without depending on the salt ^ Then ok diff --git a/ui-v2/tests/acceptance/steps/components/intention-filter-steps.js b/ui-v2/tests/acceptance/steps/components/intention-filter-steps.js new file mode 100644 index 0000000000..960cdf533d --- /dev/null +++ b/ui-v2/tests/acceptance/steps/components/intention-filter-steps.js @@ -0,0 +1,10 @@ +import steps from '../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} diff --git a/ui-v2/tests/acceptance/submit-blank.feature b/ui-v2/tests/acceptance/submit-blank.feature index a8024a384d..444c497f3e 100644 --- a/ui-v2/tests/acceptance/submit-blank.feature +++ b/ui-v2/tests/acceptance/submit-blank.feature @@ -13,11 +13,12 @@ Feature: submit blank And I submit Then the url should be /datacenter/[Slug]/create Where: - ------------------ - | Model | Slug | - | kv | kv | - | acl | acls | - ------------------ + -------------------------- + | Model | Slug | + | kv | kv | + | acl | acls | + | intention | intentions | + -------------------------- @ignore Scenario: The button is disabled Then ok diff --git a/ui-v2/tests/pages.js b/ui-v2/tests/pages.js index 599da15a38..99366a40c6 100644 --- a/ui-v2/tests/pages.js +++ b/ui-v2/tests/pages.js @@ -9,6 +9,7 @@ import kvs from 'consul-ui/tests/pages/dc/kv/index'; import kv from 'consul-ui/tests/pages/dc/kv/edit'; import acls from 'consul-ui/tests/pages/dc/acls/index'; import acl from 'consul-ui/tests/pages/dc/acls/edit'; +import intentions from 'consul-ui/tests/pages/dc/intentions/index'; import intention from 'consul-ui/tests/pages/dc/intentions/edit'; export default { @@ -23,5 +24,6 @@ export default { kv, acls, acl, + intentions, intention, }; diff --git a/ui-v2/tests/pages/components/intention-filter.js b/ui-v2/tests/pages/components/intention-filter.js new file mode 100644 index 0000000000..39a835e6d4 --- /dev/null +++ b/ui-v2/tests/pages/components/intention-filter.js @@ -0,0 +1,9 @@ +import { triggerable } from 'ember-cli-page-object'; +import radiogroup from 'consul-ui/tests/lib/page-object/radiogroup'; +export default { + ...radiogroup('action', ['', 'allow', 'deny']), + ...{ + scope: '[data-test-intention-filter]', + search: triggerable('keypress', '[name="s"]'), + }, +}; diff --git a/ui-v2/tests/pages/components/page.js b/ui-v2/tests/pages/components/page.js index 507dcaca96..aa634c7b1b 100644 --- a/ui-v2/tests/pages/components/page.js +++ b/ui-v2/tests/pages/components/page.js @@ -1,6 +1,6 @@ import { clickable } from 'ember-cli-page-object'; export default { - navigation: ['services', 'nodes', 'kvs', 'acls', 'docs', 'settings'].reduce( + navigation: ['services', 'nodes', 'kvs', 'acls', 'intentions', 'docs', 'settings'].reduce( function(prev, item, i, arr) { const key = item; return Object.assign({}, prev, { diff --git a/ui-v2/tests/pages/dc/intentions/index.js b/ui-v2/tests/pages/dc/intentions/index.js new file mode 100644 index 0000000000..b5d2a93722 --- /dev/null +++ b/ui-v2/tests/pages/dc/intentions/index.js @@ -0,0 +1,16 @@ +import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; + +import filter from 'consul-ui/tests/pages/components/intention-filter'; +export default create({ + visit: visitable('/:dc/intentions'), + intentions: collection('[data-test-tabular-row]', { + source: attribute('data-test-intention-source', '[data-test-intention-source]'), + destination: attribute('data-test-intention-destination', '[data-test-intention-destination]'), + action: attribute('data-test-intention-action', '[data-test-intention-action]'), + intention: clickable('a'), + actions: clickable('label'), + delete: clickable('[data-test-delete]'), + confirmDelete: clickable('button.type-delete'), + }), + filter: filter, +}); diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index 901843bd12..2a61c74555 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -5,6 +5,8 @@ import getDictionary from '@hashicorp/ember-cli-api-double/dictionary'; import pages from 'consul-ui/tests/pages'; import api from 'consul-ui/tests/helpers/api'; +const dont = `( don't| shouldn't| can't)?`; + const create = function(number, name, value) { // don't return a promise here as // I don't need it to wait @@ -240,7 +242,9 @@ export default function(assert) { assert.equal(len, num, `Expected ${num} ${model}s, saw ${len}`); }) - .then(['I see $num $model model with the $property "$value"'], function( + // TODO: I${ dont } see + .then([`I see $num $model model[s]? with the $property "$value"`], function( + // negate, num, model, property, From e3ce2a8bebef9021a0ccab87bea7e0676bc05119 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 13:41:44 +0100 Subject: [PATCH 27/54] Lock Session invalidation acceptance test --- ui-v2/app/templates/dc/nodes/-sessions.hbs | 2 +- .../dc/nodes/sessions/invalidate.feature | 26 +++++++++++++++++++ .../acceptance/dc/nodes/sessions/list.feature | 2 +- .../dc/nodes/sessions/invalidate-steps.js | 10 +++++++ ui-v2/tests/pages/dc/nodes/show.js | 4 ++- ui-v2/tests/steps.js | 2 +- 6 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature create mode 100644 ui-v2/tests/acceptance/steps/dc/nodes/sessions/invalidate-steps.js diff --git a/ui-v2/app/templates/dc/nodes/-sessions.hbs b/ui-v2/app/templates/dc/nodes/-sessions.hbs index 8e3d43ddbc..5c0cfda925 100644 --- a/ui-v2/app/templates/dc/nodes/-sessions.hbs +++ b/ui-v2/app/templates/dc/nodes/-sessions.hbs @@ -37,7 +37,7 @@ {{#confirmation-dialog message='Are you sure you want to invalidate this session?'}} {{#block-slot 'action' as |confirm|}} - + {{/block-slot}} {{#block-slot 'dialog' as |execute cancel message|}}

diff --git a/ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature b/ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature new file mode 100644 index 0000000000..786b9e4fd7 --- /dev/null +++ b/ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature @@ -0,0 +1,26 @@ +@setupApplicationTest +Feature: dc / nodes / sessions / invalidate: Invalidate Lock Sessions + In order to invalidate a lock session + As a user + I should be able to invalidate a lock session by clicking a button and confirming + Scenario: Given 2 lock sessions + Given 1 datacenter model with the value "dc1" + And 1 node model from yaml + --- + - ID: node-0 + --- + And 2 session models from yaml + --- + - ID: 7bbbd8bb-fff3-4292-b6e3-cfedd788546a + - ID: 7ccd0bd7-a5e0-41ae-a33e-ed3793d803b2 + --- + When I visit the node page for yaml + --- + dc: dc1 + node: node-0 + --- + And I click lockSessions on the tabs + Then I see lockSessionsIsSelected on the tabs + And I click delete on the sessions + And I click confirmDelete on the sessions + Then a PUT request is made to "/v1/session/destroy/7bbbd8bb-fff3-4292-b6e3-cfedd788546a?dc=dc1" diff --git a/ui-v2/tests/acceptance/dc/nodes/sessions/list.feature b/ui-v2/tests/acceptance/dc/nodes/sessions/list.feature index 41055dc745..4c7f17beda 100644 --- a/ui-v2/tests/acceptance/dc/nodes/sessions/list.feature +++ b/ui-v2/tests/acceptance/dc/nodes/sessions/list.feature @@ -1,5 +1,5 @@ @setupApplicationTest -Feature: dc / nodes / sessions /list: List Lock Sessions +Feature: dc / nodes / sessions / list: List Lock Sessions In order to get information regarding lock sessions As a user I should be able to see a listing of lock sessions with necessary information under the lock sessions tab for a node diff --git a/ui-v2/tests/acceptance/steps/dc/nodes/sessions/invalidate-steps.js b/ui-v2/tests/acceptance/steps/dc/nodes/sessions/invalidate-steps.js new file mode 100644 index 0000000000..9bfbe9ac9b --- /dev/null +++ b/ui-v2/tests/acceptance/steps/dc/nodes/sessions/invalidate-steps.js @@ -0,0 +1,10 @@ +import steps from '../../../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} diff --git a/ui-v2/tests/pages/dc/nodes/show.js b/ui-v2/tests/pages/dc/nodes/show.js index d51bda6cf5..298c51f543 100644 --- a/ui-v2/tests/pages/dc/nodes/show.js +++ b/ui-v2/tests/pages/dc/nodes/show.js @@ -1,4 +1,4 @@ -import { create, visitable, collection, attribute } from 'ember-cli-page-object'; +import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; import radiogroup from 'consul-ui/tests/lib/page-object/radiogroup'; export default create({ @@ -11,6 +11,8 @@ export default create({ port: attribute('data-test-service-port', '.port'), }), sessions: collection('#lock-sessions [data-test-tabular-row]', { + delete: clickable('[data-test-delete]'), + confirmDelete: clickable('button.type-delete'), TTL: attribute('data-test-session-ttl', '[data-test-session-ttl]'), }), }); diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index 2a61c74555..a3d9365ab5 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -5,7 +5,7 @@ import getDictionary from '@hashicorp/ember-cli-api-double/dictionary'; import pages from 'consul-ui/tests/pages'; import api from 'consul-ui/tests/helpers/api'; -const dont = `( don't| shouldn't| can't)?`; +// const dont = `( don't| shouldn't| can't)?`; const create = function(number, name, value) { // don't return a promise here as From e0f7cdb1287fcf0d58c1515824f21a26b4fd177f Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 15:06:20 +0100 Subject: [PATCH 28/54] Start purifying page objects --- ui-v2/app/templates/dc/acls/-form.hbs | 2 +- ui-v2/tests/acceptance/dc/acls/delete.feature | 17 +++++++- .../tests/lib/page-object/createDeletable.js | 11 +++++ .../tests/lib/page-object/createSubmitable.js | 11 +++++ ui-v2/tests/pages.js | 41 +++++++++++++------ ui-v2/tests/pages/dc.js | 21 ++++++---- ui-v2/tests/pages/dc/acls/edit.js | 18 ++++---- ui-v2/tests/pages/dc/acls/index.js | 28 ++++++------- ui-v2/tests/pages/dc/intentions/edit.js | 15 ++++--- ui-v2/tests/pages/dc/intentions/index.js | 35 ++++++++-------- ui-v2/tests/pages/dc/kv/edit.js | 18 ++++---- ui-v2/tests/pages/dc/kv/index.js | 25 +++++------ ui-v2/tests/pages/dc/nodes/index.js | 21 +++++----- ui-v2/tests/pages/dc/nodes/show.js | 36 ++++++++-------- ui-v2/tests/pages/dc/services/index.js | 28 ++++++------- ui-v2/tests/pages/dc/services/show.js | 35 ++++++++-------- ui-v2/tests/pages/index.js | 12 +++--- ui-v2/tests/pages/settings.js | 11 +++-- ui-v2/tests/steps.js | 9 ++++ 19 files changed, 223 insertions(+), 171 deletions(-) create mode 100644 ui-v2/tests/lib/page-object/createDeletable.js create mode 100644 ui-v2/tests/lib/page-object/createSubmitable.js diff --git a/ui-v2/app/templates/dc/acls/-form.hbs b/ui-v2/app/templates/dc/acls/-form.hbs index 2d0ef5edf1..803444e5a2 100644 --- a/ui-v2/app/templates/dc/acls/-form.hbs +++ b/ui-v2/app/templates/dc/acls/-form.hbs @@ -35,7 +35,7 @@ {{# if (and (not create) (not-eq item.ID 'anonymous')) }} {{#confirmation-dialog message='Are you sure you want to delete this ACL token?'}} {{#block-slot 'action' as |confirm|}} - + {{/block-slot}} {{#block-slot 'dialog' as |execute cancel message|}}

diff --git a/ui-v2/tests/acceptance/dc/acls/delete.feature b/ui-v2/tests/acceptance/dc/acls/delete.feature index 9a59d1ec0b..4842b2c712 100644 --- a/ui-v2/tests/acceptance/dc/acls/delete.feature +++ b/ui-v2/tests/acceptance/dc/acls/delete.feature @@ -1,6 +1,6 @@ @setupApplicationTest Feature: dc / acls / delete: ACL Delete - Scenario: Delete ACL + Scenario: Deleting an ACL from the ACL listing page Given 1 datacenter model with the value "datacenter" And 1 acl model from yaml --- @@ -15,3 +15,18 @@ Feature: dc / acls / delete: ACL Delete And I click delete on the acls And I click confirmDelete on the acls Then a PUT request is made to "/v1/acl/destroy/key?dc=datacenter" + Scenario: Deleting an ACL from the ACL detail page + Given 1 datacenter model with the value "datacenter" + And 1 acl model from yaml + --- + Name: something + ID: key + --- + When I visit the acl page for yaml + --- + dc: datacenter + acl: something + --- + And I click delete + And I click confirmDelete + Then a PUT request is made to "/v1/acl/destroy/something?dc=datacenter" diff --git a/ui-v2/tests/lib/page-object/createDeletable.js b/ui-v2/tests/lib/page-object/createDeletable.js new file mode 100644 index 0000000000..cc3aff4c59 --- /dev/null +++ b/ui-v2/tests/lib/page-object/createDeletable.js @@ -0,0 +1,11 @@ +export default function(clickable) { + return function(obj) { + return { + ...obj, + ...{ + delete: clickable('[data-test-delete]'), + confirmDelete: clickable('button.type-delete'), + }, + }; + }; +} diff --git a/ui-v2/tests/lib/page-object/createSubmitable.js b/ui-v2/tests/lib/page-object/createSubmitable.js new file mode 100644 index 0000000000..c566bd7f06 --- /dev/null +++ b/ui-v2/tests/lib/page-object/createSubmitable.js @@ -0,0 +1,11 @@ +export default function(clickable, is) { + return function(obj) { + return { + ...obj, + ...{ + submit: clickable('[type=submit]'), + submitIsEnabled: is(':not(:disabled)', '[type=submit]'), + }, + }; + }; +} diff --git a/ui-v2/tests/pages.js b/ui-v2/tests/pages.js index 99366a40c6..175a4dd1c7 100644 --- a/ui-v2/tests/pages.js +++ b/ui-v2/tests/pages.js @@ -1,29 +1,44 @@ +import { create, clickable, is, attribute, collection, text } from 'ember-cli-page-object'; +import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; +import createDeletable from 'consul-ui/tests/lib/page-object/createDeletable'; +import createSubmitable from 'consul-ui/tests/lib/page-object/createSubmitable'; + +import page from 'consul-ui/tests/pages/components/page'; +import radiogroup from 'consul-ui/tests/lib/page-object/radiogroup'; + import index from 'consul-ui/tests/pages/index'; import dcs from 'consul-ui/tests/pages/dc'; import settings from 'consul-ui/tests/pages/settings'; +import catalogFilter from 'consul-ui/tests/pages/components/catalog-filter'; import services from 'consul-ui/tests/pages/dc/services/index'; import service from 'consul-ui/tests/pages/dc/services/show'; import nodes from 'consul-ui/tests/pages/dc/nodes/index'; import node from 'consul-ui/tests/pages/dc/nodes/show'; import kvs from 'consul-ui/tests/pages/dc/kv/index'; import kv from 'consul-ui/tests/pages/dc/kv/edit'; +import aclFilter from 'consul-ui/tests/pages/components/acl-filter'; import acls from 'consul-ui/tests/pages/dc/acls/index'; import acl from 'consul-ui/tests/pages/dc/acls/edit'; +import intentionFilter from 'consul-ui/tests/pages/components/intention-filter'; import intentions from 'consul-ui/tests/pages/dc/intentions/index'; import intention from 'consul-ui/tests/pages/dc/intentions/edit'; +const deletable = createDeletable(clickable); +const submitable = createSubmitable(clickable, is); export default { - index, - dcs, - settings, - services, - service, - nodes, - node, - kvs, - kv, - acls, - acl, - intentions, - intention, + index: create(index(visitable, collection)), + dcs: create(dcs(visitable, clickable, attribute, collection)), + services: create(services(visitable, clickable, attribute, collection, page, catalogFilter)), + service: create(service(visitable, attribute, collection, text, catalogFilter)), + nodes: create(nodes(visitable, clickable, attribute, collection, catalogFilter)), + node: create(node(visitable, deletable, clickable, attribute, collection, radiogroup)), + kvs: create(kvs(visitable, deletable, clickable, attribute, collection)), + kv: create(kv(visitable, submitable, deletable)), + acls: create(acls(visitable, deletable, clickable, attribute, collection, aclFilter)), + acl: create(acl(visitable, submitable, deletable)), + intentions: create( + intentions(visitable, deletable, clickable, attribute, collection, intentionFilter) + ), + intention: create(intention(visitable, submitable, deletable)), + settings: create(settings(visitable, submitable)), }; diff --git a/ui-v2/tests/pages/dc.js b/ui-v2/tests/pages/dc.js index ad4eeb9a3b..8517a747d2 100644 --- a/ui-v2/tests/pages/dc.js +++ b/ui-v2/tests/pages/dc.js @@ -1,9 +1,12 @@ -import { create, visitable, attribute, collection, clickable } from 'ember-cli-page-object'; - -export default create({ - visit: visitable('/:dc/'), - dcs: collection('[data-test-datacenter-picker]'), - showDatacenters: clickable('[data-test-datacenter-selected]'), - selectedDc: attribute('data-test-datacenter-selected', '[data-test-datacenter-selected]'), - selectedDatacenter: attribute('data-test-datacenter-selected', '[data-test-datacenter-selected]'), -}); +export default function(visitable, clickable, attribute, collection) { + return { + visit: visitable('/:dc/'), + dcs: collection('[data-test-datacenter-picker]'), + showDatacenters: clickable('[data-test-datacenter-selected]'), + selectedDc: attribute('data-test-datacenter-selected', '[data-test-datacenter-selected]'), + selectedDatacenter: attribute( + 'data-test-datacenter-selected', + '[data-test-datacenter-selected]' + ), + }; +} diff --git a/ui-v2/tests/pages/dc/acls/edit.js b/ui-v2/tests/pages/dc/acls/edit.js index 7056072201..1164d4c0ce 100644 --- a/ui-v2/tests/pages/dc/acls/edit.js +++ b/ui-v2/tests/pages/dc/acls/edit.js @@ -1,11 +1,7 @@ -import { create, clickable, triggerable, is } from 'ember-cli-page-object'; -import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; - -export default create({ - // custom visitable - visit: visitable(['/:dc/acls/:acl', '/:dc/acls/create']), - // fillIn: fillable('input, textarea, [contenteditable]'), - name: triggerable('keypress', '[name="name"]'), - submit: clickable('[type=submit]'), - submitIsEnabled: is(':not(:disabled)', '[type=submit]'), -}); +export default function(visitable, submitable, deletable, triggerable) { + return submitable( + deletable({ + visit: visitable(['/:dc/acls/:acl', '/:dc/acls/create']), + }) + ); +} diff --git a/ui-v2/tests/pages/dc/acls/index.js b/ui-v2/tests/pages/dc/acls/index.js index 8b3c60cc5d..7c567840e1 100644 --- a/ui-v2/tests/pages/dc/acls/index.js +++ b/ui-v2/tests/pages/dc/acls/index.js @@ -1,14 +1,14 @@ -import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; - -import filter from 'consul-ui/tests/pages/components/acl-filter'; -export default create({ - visit: visitable('/:dc/acls'), - acls: collection('[data-test-tabular-row]', { - name: attribute('data-test-acl', '[data-test-acl]'), - acl: clickable('a'), - actions: clickable('label'), - delete: clickable('[data-test-delete]'), - confirmDelete: clickable('button.type-delete'), - }), - filter: filter, -}); +export default function(visitable, deletable, clickable, attribute, collection, filter) { + return { + visit: visitable('/:dc/acls'), + acls: collection( + '[data-test-tabular-row]', + deletable({ + name: attribute('data-test-acl', '[data-test-acl]'), + acl: clickable('a'), + actions: clickable('label'), + }) + ), + filter: filter, + }; +} diff --git a/ui-v2/tests/pages/dc/intentions/edit.js b/ui-v2/tests/pages/dc/intentions/edit.js index d054ff24fc..5c6d94550a 100644 --- a/ui-v2/tests/pages/dc/intentions/edit.js +++ b/ui-v2/tests/pages/dc/intentions/edit.js @@ -1,8 +1,7 @@ -import { create, clickable } from 'ember-cli-page-object'; -import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; - -export default create({ - // custom visitable - visit: visitable(['/:dc/intentions/:intention', '/:dc/intentions/create']), - submit: clickable('[type=submit]'), -}); +export default function(visitable, submitable, deletable) { + return submitable( + deletable({ + visit: visitable(['/:dc/intentions/:intention', '/:dc/intentions/create']), + }) + ); +} diff --git a/ui-v2/tests/pages/dc/intentions/index.js b/ui-v2/tests/pages/dc/intentions/index.js index b5d2a93722..5ab14b01b0 100644 --- a/ui-v2/tests/pages/dc/intentions/index.js +++ b/ui-v2/tests/pages/dc/intentions/index.js @@ -1,16 +1,19 @@ -import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; - -import filter from 'consul-ui/tests/pages/components/intention-filter'; -export default create({ - visit: visitable('/:dc/intentions'), - intentions: collection('[data-test-tabular-row]', { - source: attribute('data-test-intention-source', '[data-test-intention-source]'), - destination: attribute('data-test-intention-destination', '[data-test-intention-destination]'), - action: attribute('data-test-intention-action', '[data-test-intention-action]'), - intention: clickable('a'), - actions: clickable('label'), - delete: clickable('[data-test-delete]'), - confirmDelete: clickable('button.type-delete'), - }), - filter: filter, -}); +export default function(visitable, deletable, clickable, attribute, collection, filter) { + return { + visit: visitable('/:dc/intentions'), + intentions: collection( + '[data-test-tabular-row]', + deletable({ + source: attribute('data-test-intention-source', '[data-test-intention-source]'), + destination: attribute( + 'data-test-intention-destination', + '[data-test-intention-destination]' + ), + action: attribute('data-test-intention-action', '[data-test-intention-action]'), + intention: clickable('a'), + actions: clickable('label'), + }) + ), + filter: filter, + }; +} diff --git a/ui-v2/tests/pages/dc/kv/edit.js b/ui-v2/tests/pages/dc/kv/edit.js index df07d0df5b..f7a019705b 100644 --- a/ui-v2/tests/pages/dc/kv/edit.js +++ b/ui-v2/tests/pages/dc/kv/edit.js @@ -1,11 +1,7 @@ -import { create, clickable, is } from 'ember-cli-page-object'; -import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; - -export default create({ - // custom visitable - visit: visitable(['/:dc/kv/:kv/edit', '/:dc/kv/create'], str => str), - // fillIn: fillable('input, textarea, [contenteditable]'), - // name: triggerable('keypress', '[name="additional"]'), - submit: clickable('[type=submit]'), - submitIsEnabled: is(':not(:disabled)', '[type=submit]'), -}); +export default function(visitable, submitable, deletable) { + return submitable( + deletable({ + visit: visitable(['/:dc/kv/:kv/edit', '/:dc/kv/create'], str => str), + }) + ); +} diff --git a/ui-v2/tests/pages/dc/kv/index.js b/ui-v2/tests/pages/dc/kv/index.js index 446122e6c3..a0c8439145 100644 --- a/ui-v2/tests/pages/dc/kv/index.js +++ b/ui-v2/tests/pages/dc/kv/index.js @@ -1,12 +1,13 @@ -import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; - -export default create({ - visit: visitable('/:dc/kv'), - kvs: collection('[data-test-tabular-row]', { - name: attribute('data-test-kv', '[data-test-kv]'), - kv: clickable('a'), - actions: clickable('label'), - delete: clickable('[data-test-delete]'), - confirmDelete: clickable('button.type-delete'), - }), -}); +export default function(visitable, deletable, clickable, attribute, collection) { + return { + visit: visitable('/:dc/kv'), + kvs: collection( + '[data-test-tabular-row]', + deletable({ + name: attribute('data-test-kv', '[data-test-kv]'), + kv: clickable('a'), + actions: clickable('label'), + }) + ), + }; +} diff --git a/ui-v2/tests/pages/dc/nodes/index.js b/ui-v2/tests/pages/dc/nodes/index.js index 963186306b..1899e7df5e 100644 --- a/ui-v2/tests/pages/dc/nodes/index.js +++ b/ui-v2/tests/pages/dc/nodes/index.js @@ -1,11 +1,10 @@ -import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; -import filter from 'consul-ui/tests/pages/components/catalog-filter'; - -export default create({ - visit: visitable('/:dc/nodes'), - nodes: collection('[data-test-node]', { - name: attribute('data-test-node'), - node: clickable('header a'), - }), - filter: filter, -}); +export default function(visitable, clickable, attribute, collection, filter) { + return { + visit: visitable('/:dc/nodes'), + nodes: collection('[data-test-node]', { + name: attribute('data-test-node'), + node: clickable('header a'), + }), + filter: filter, + }; +} diff --git a/ui-v2/tests/pages/dc/nodes/show.js b/ui-v2/tests/pages/dc/nodes/show.js index 298c51f543..4296397685 100644 --- a/ui-v2/tests/pages/dc/nodes/show.js +++ b/ui-v2/tests/pages/dc/nodes/show.js @@ -1,18 +1,18 @@ -import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; - -import radiogroup from 'consul-ui/tests/lib/page-object/radiogroup'; -export default create({ - visit: visitable('/:dc/nodes/:node'), - tabs: radiogroup('tab', ['health-checks', 'services', 'round-trip-time', 'lock-sessions']), - healthchecks: collection('[data-test-node-healthcheck]', { - name: attribute('data-test-node-healthcheck'), - }), - services: collection('#services [data-test-tabular-row]', { - port: attribute('data-test-service-port', '.port'), - }), - sessions: collection('#lock-sessions [data-test-tabular-row]', { - delete: clickable('[data-test-delete]'), - confirmDelete: clickable('button.type-delete'), - TTL: attribute('data-test-session-ttl', '[data-test-session-ttl]'), - }), -}); +export default function(visitable, deletable, clickable, attribute, collection, radiogroup) { + return { + visit: visitable('/:dc/nodes/:node'), + tabs: radiogroup('tab', ['health-checks', 'services', 'round-trip-time', 'lock-sessions']), + healthchecks: collection('[data-test-node-healthcheck]', { + name: attribute('data-test-node-healthcheck'), + }), + services: collection('#services [data-test-tabular-row]', { + port: attribute('data-test-service-port', '.port'), + }), + sessions: collection( + '#lock-sessions [data-test-tabular-row]', + deletable({ + TTL: attribute('data-test-session-ttl', '[data-test-session-ttl]'), + }) + ), + }; +} diff --git a/ui-v2/tests/pages/dc/services/index.js b/ui-v2/tests/pages/dc/services/index.js index acfd554ce3..d403b0b10b 100644 --- a/ui-v2/tests/pages/dc/services/index.js +++ b/ui-v2/tests/pages/dc/services/index.js @@ -1,16 +1,12 @@ -import { create, visitable, collection, attribute, clickable } from 'ember-cli-page-object'; - -import page from 'consul-ui/tests/pages/components/page'; -import filter from 'consul-ui/tests/pages/components/catalog-filter'; - -export default create({ - visit: visitable('/:dc/services'), - services: collection('[data-test-service]', { - name: attribute('data-test-service'), - service: clickable('a'), - }), - dcs: collection('[data-test-datacenter-picker]'), - navigation: page.navigation, - - filter: filter, -}); +export default function(visitable, clickable, attribute, collection, page, filter) { + return { + visit: visitable('/:dc/services'), + services: collection('[data-test-service]', { + name: attribute('data-test-service'), + service: clickable('a'), + }), + dcs: collection('[data-test-datacenter-picker]'), + navigation: page.navigation, + filter: filter, + }; +} diff --git a/ui-v2/tests/pages/dc/services/show.js b/ui-v2/tests/pages/dc/services/show.js index b98a3655eb..483e3a4127 100644 --- a/ui-v2/tests/pages/dc/services/show.js +++ b/ui-v2/tests/pages/dc/services/show.js @@ -1,18 +1,17 @@ -import { create, visitable, collection, attribute, text } from 'ember-cli-page-object'; -import filter from 'consul-ui/tests/pages/components/catalog-filter'; - -export default create({ - visit: visitable('/:dc/services/:service'), - nodes: collection('[data-test-node]', { - name: attribute('data-test-node'), - }), - healthy: collection('[data-test-healthy] [data-test-node]', { - name: attribute('data-test-node'), - address: text('header strong'), - }), - unhealthy: collection('[data-test-unhealthy] [data-test-node]', { - name: attribute('data-test-node'), - address: text('header strong'), - }), - filter: filter, -}); +export default function(visitable, attribute, collection, text, filter) { + return { + visit: visitable('/:dc/services/:service'), + nodes: collection('[data-test-node]', { + name: attribute('data-test-node'), + }), + healthy: collection('[data-test-healthy] [data-test-node]', { + name: attribute('data-test-node'), + address: text('header strong'), + }), + unhealthy: collection('[data-test-unhealthy] [data-test-node]', { + name: attribute('data-test-node'), + address: text('header strong'), + }), + filter: filter, + }; +} diff --git a/ui-v2/tests/pages/index.js b/ui-v2/tests/pages/index.js index 28842ef3bc..d61dc27bce 100644 --- a/ui-v2/tests/pages/index.js +++ b/ui-v2/tests/pages/index.js @@ -1,6 +1,6 @@ -import { create, visitable, collection } from 'ember-cli-page-object'; - -export default create({ - visit: visitable('/'), - dcs: collection('[data-test-datacenter-list]'), -}); +export default function(visitable, collection) { + return { + visit: visitable('/'), + dcs: collection('[data-test-datacenter-list]'), + }; +} diff --git a/ui-v2/tests/pages/settings.js b/ui-v2/tests/pages/settings.js index 0418b03853..6d94078808 100644 --- a/ui-v2/tests/pages/settings.js +++ b/ui-v2/tests/pages/settings.js @@ -1,6 +1,5 @@ -import { create, visitable, clickable } from 'ember-cli-page-object'; - -export default create({ - visit: visitable('/settings'), - submit: clickable('[type=submit]'), -}); +export default function(visitable, submitable) { + return submitable({ + visit: visitable('/settings'), + }); +} diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index a3d9365ab5..b8d0a732f0 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -85,6 +85,15 @@ export default function(assert) { .when('I click "$selector"', function(selector) { return click(selector); }) + // TODO: Probably nicer to thing of better vocab than having the 'without " rule' + .when('I click (?!")$property(?!")', function(property) { + try { + return currentPage[property](); + } catch (e) { + console.error(e); + throw new Error(`The '${property}' property on the page object doesn't exist`); + } + }) .when('I click $prop on the $component', function(prop, component) { // Collection var obj; From 701d6a3a722145ef902d32dc1647e4938f414ae2 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 15:58:09 +0100 Subject: [PATCH 29/54] Move deleting to a top level feature --- ui-v2/app/templates/dc/intentions/-form.hbs | 2 +- ui-v2/app/templates/dc/intentions/index.hbs | 2 +- ui-v2/app/templates/dc/kv/-form.hbs | 2 +- ui-v2/package.json | 2 +- ui-v2/tests/acceptance/dc/acls/delete.feature | 32 ----------------- ui-v2/tests/acceptance/dc/kvs/delete.feature | 16 --------- ui-v2/tests/acceptance/deleting.feature | 34 +++++++++++++++++++ .../acceptance/steps/dc/kvs/delete-steps.js | 10 ------ .../delete-steps.js => deleting-steps.js} | 9 ++--- 9 files changed, 43 insertions(+), 66 deletions(-) delete mode 100644 ui-v2/tests/acceptance/dc/acls/delete.feature delete mode 100644 ui-v2/tests/acceptance/dc/kvs/delete.feature create mode 100644 ui-v2/tests/acceptance/deleting.feature delete mode 100644 ui-v2/tests/acceptance/steps/dc/kvs/delete-steps.js rename ui-v2/tests/acceptance/steps/{dc/acls/delete-steps.js => deleting-steps.js} (50%) diff --git a/ui-v2/app/templates/dc/intentions/-form.hbs b/ui-v2/app/templates/dc/intentions/-form.hbs index a00d1d5caa..6a4507cc8b 100644 --- a/ui-v2/app/templates/dc/intentions/-form.hbs +++ b/ui-v2/app/templates/dc/intentions/-form.hbs @@ -64,7 +64,7 @@ {{# if (and item.ID (not-eq item.ID 'anonymous')) }} {{#confirmation-dialog message='Are you sure you want to delete this Intention?'}} {{#block-slot 'action' as |confirm|}} - + {{/block-slot}} {{#block-slot 'dialog' as |execute cancel message|}}

diff --git a/ui-v2/app/templates/dc/intentions/index.hbs b/ui-v2/app/templates/dc/intentions/index.hbs index 946524d05f..edbb49d6f9 100644 --- a/ui-v2/app/templates/dc/intentions/index.hbs +++ b/ui-v2/app/templates/dc/intentions/index.hbs @@ -58,7 +58,7 @@ Edit

  • - Delete + Delete
    • {{/action-group}} diff --git a/ui-v2/app/templates/dc/kv/-form.hbs b/ui-v2/app/templates/dc/kv/-form.hbs index 9b859c5a19..fa188f9d4a 100644 --- a/ui-v2/app/templates/dc/kv/-form.hbs +++ b/ui-v2/app/templates/dc/kv/-form.hbs @@ -33,7 +33,7 @@ {{#confirmation-dialog message='Are you sure you want to delete this key?'}} {{#block-slot 'action' as |confirm|}} - + {{/block-slot}} {{#block-slot 'dialog' as |execute cancel message|}}

    diff --git a/ui-v2/package.json b/ui-v2/package.json index 5dd0dee53b..70be6565be 100644 --- a/ui-v2/package.json +++ b/ui-v2/package.json @@ -2,7 +2,7 @@ "name": "consul-ui", "version": "2.2.0", "private": true, - "description": "The web ui for Consul, by HashiCorp.", + "description": "The web UI for Consul, by HashiCorp.", "directories": { "doc": "doc", "test": "tests" diff --git a/ui-v2/tests/acceptance/dc/acls/delete.feature b/ui-v2/tests/acceptance/dc/acls/delete.feature deleted file mode 100644 index 4842b2c712..0000000000 --- a/ui-v2/tests/acceptance/dc/acls/delete.feature +++ /dev/null @@ -1,32 +0,0 @@ -@setupApplicationTest -Feature: dc / acls / delete: ACL Delete - Scenario: Deleting an ACL from the ACL listing page - Given 1 datacenter model with the value "datacenter" - And 1 acl model from yaml - --- - Name: something - ID: key - --- - When I visit the acls page for yaml - --- - dc: datacenter - --- - And I click actions on the acls - And I click delete on the acls - And I click confirmDelete on the acls - Then a PUT request is made to "/v1/acl/destroy/key?dc=datacenter" - Scenario: Deleting an ACL from the ACL detail page - Given 1 datacenter model with the value "datacenter" - And 1 acl model from yaml - --- - Name: something - ID: key - --- - When I visit the acl page for yaml - --- - dc: datacenter - acl: something - --- - And I click delete - And I click confirmDelete - Then a PUT request is made to "/v1/acl/destroy/something?dc=datacenter" diff --git a/ui-v2/tests/acceptance/dc/kvs/delete.feature b/ui-v2/tests/acceptance/dc/kvs/delete.feature deleted file mode 100644 index 324240b9c9..0000000000 --- a/ui-v2/tests/acceptance/dc/kvs/delete.feature +++ /dev/null @@ -1,16 +0,0 @@ -@setupApplicationTest -Feature: dc / kvs / delete: KV Delete - Scenario: Delete ACL - Given 1 datacenter model with the value "datacenter" - And 1 kv model from yaml - --- - - key-name - --- - When I visit the kvs page for yaml - --- - dc: datacenter - --- - And I click actions on the kvs - And I click delete on the kvs - And I click confirmDelete on the kvs - Then a DELETE request is made to "/v1/kv/key-name?dc=datacenter" diff --git a/ui-v2/tests/acceptance/deleting.feature b/ui-v2/tests/acceptance/deleting.feature new file mode 100644 index 0000000000..30808f77cd --- /dev/null +++ b/ui-v2/tests/acceptance/deleting.feature @@ -0,0 +1,34 @@ +@setupApplicationTest +Feature: deleting: Deleting form the listing and the detail page with confirmation + Scenario: Deleting a [Model] from the [Model] listing page + Given 1 datacenter model with the value "datacenter" + And 1 [Model] model from json + --- + [Data] + --- + When I visit the [Model]s page for yaml + --- + dc: datacenter + --- + And I click actions on the [Model]s + And I click delete on the [Model]s + And I click confirmDelete on the [Model]s + Then a [Method] request is made to "[URL]" + When I visit the [Model] page for yaml + --- + dc: datacenter + [Slug] + --- + And I click delete + And I click confirmDelete + Then a [Method] request is made to "[URL]" + Where: + --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + | Model | Method | URL | Data | Slug | + | acl | PUT | /v1/acl/destroy/something?dc=datacenter | {"Name": "something", "ID": "something"} | acl: something | + | kv | DELETE | /v1/kv/key-name?dc=datacenter | ["key-name"] | kv: key-name | + | intention | DELETE | /v1/connect/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=datacenter | {"SourceName": "name", "ID": "ee52203d-989f-4f7a-ab5a-2bef004164ca"} | intention: ee52203d-989f-4f7a-ab5a-2bef004164ca | + --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +@ignore + Scenario: Sort out the wide tables ^ + Then ok diff --git a/ui-v2/tests/acceptance/steps/dc/kvs/delete-steps.js b/ui-v2/tests/acceptance/steps/dc/kvs/delete-steps.js deleted file mode 100644 index a7eff3228b..0000000000 --- a/ui-v2/tests/acceptance/steps/dc/kvs/delete-steps.js +++ /dev/null @@ -1,10 +0,0 @@ -import steps from '../../steps'; - -// step definitions that are shared between features should be moved to the -// tests/acceptance/steps/steps.js file - -export default function(assert) { - return steps(assert).then('I should find a file', function() { - assert.ok(true, this.step); - }); -} diff --git a/ui-v2/tests/acceptance/steps/dc/acls/delete-steps.js b/ui-v2/tests/acceptance/steps/deleting-steps.js similarity index 50% rename from ui-v2/tests/acceptance/steps/dc/acls/delete-steps.js rename to ui-v2/tests/acceptance/steps/deleting-steps.js index a7eff3228b..6e459ed1f0 100644 --- a/ui-v2/tests/acceptance/steps/dc/acls/delete-steps.js +++ b/ui-v2/tests/acceptance/steps/deleting-steps.js @@ -1,10 +1,11 @@ -import steps from '../../steps'; +import steps from './steps'; // step definitions that are shared between features should be moved to the // tests/acceptance/steps/steps.js file export default function(assert) { - return steps(assert).then('I should find a file', function() { - assert.ok(true, this.step); - }); + return steps(assert) + .then('I should find a file', function() { + assert.ok(true, this.step); + }); } From 45bb8bce709d6199c94cf3ed2c9acebfa80dcdfd Mon Sep 17 00:00:00 2001 From: M S Vishwanath Bhat Date: Wed, 4 Jul 2018 17:07:24 +0200 Subject: [PATCH 30/54] Trivial spell correction in connect configuration doc Signed-off-by: M S Vishwanath Bhat --- website/source/docs/connect/configuration.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/connect/configuration.html.md b/website/source/docs/connect/configuration.html.md index a7f9926705..3a08348810 100644 --- a/website/source/docs/connect/configuration.html.md +++ b/website/source/docs/connect/configuration.html.md @@ -90,7 +90,7 @@ described here, the rest of the service definition is shown for context and is * `bind_port` - The port the proxy will bind it's _public_ mTLS listener to. If not provided, the - agent will attempt to assing one from its [configured proxy port + agent will attempt to assign one from its [configured proxy port range](/docs/agent/options.html#proxy_min_port) if available. By default the range is [20000, 20255] and the port is selected at random from that range. From 649d7777145bcef397e6e01db7be2843b84ea202 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 16:37:41 +0100 Subject: [PATCH 31/54] Acceptance test for using tokens from listing and detail pages --- ui-v2/app/templates/dc/acls/edit.hbs | 2 +- ui-v2/tests/acceptance/dc/acls/use.feature | 40 +++++++++++++++++++ .../tests/acceptance/settings/update.feature | 4 ++ .../acceptance/steps/dc/acls/use-steps.js | 10 +++++ ui-v2/tests/pages.js | 2 +- ui-v2/tests/pages/dc/acls/edit.js | 4 +- ui-v2/tests/pages/dc/acls/index.js | 2 + 7 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 ui-v2/tests/acceptance/dc/acls/use.feature create mode 100644 ui-v2/tests/acceptance/steps/dc/acls/use-steps.js diff --git a/ui-v2/app/templates/dc/acls/edit.hbs b/ui-v2/app/templates/dc/acls/edit.hbs index 9ab41201bd..e63de26726 100644 --- a/ui-v2/app/templates/dc/acls/edit.hbs +++ b/ui-v2/app/templates/dc/acls/edit.hbs @@ -35,7 +35,7 @@ {{#confirmation-dialog message='Are you sure you want to use this ACL token?'}} {{#block-slot 'action' as |confirm|}} - + {{/block-slot}} {{#block-slot 'dialog' as |execute cancel message|}}

    diff --git a/ui-v2/tests/acceptance/dc/acls/use.feature b/ui-v2/tests/acceptance/dc/acls/use.feature new file mode 100644 index 0000000000..6ad7224872 --- /dev/null +++ b/ui-v2/tests/acceptance/dc/acls/use.feature @@ -0,0 +1,40 @@ +@setupApplicationTest +Feature: dc / acls / use: Using an ACL token + Background: + Given 1 datacenter model with the value "datacenter" + And 1 acl model from yaml + --- + ID: token + --- + Scenario: Using an ACL token from the listing page + When I visit the acls page for yaml + --- + dc: datacenter + --- + Then I have settings like yaml + --- + token: ~ + --- + And I click actions on the acls + And I click use on the acls + And I click confirmUse on the acls + Then I have settings like yaml + --- + token: token + --- + Scenario: Using an ACL token from the detail page + When I visit the acl page for yaml + --- + dc: datacenter + acl: token + --- + Then I have settings like yaml + --- + token: ~ + --- + And I click use + And I click confirmUse + Then I have settings like yaml + --- + token: token + --- diff --git a/ui-v2/tests/acceptance/settings/update.feature b/ui-v2/tests/acceptance/settings/update.feature index 98ac584d0b..300a9123a9 100644 --- a/ui-v2/tests/acceptance/settings/update.feature +++ b/ui-v2/tests/acceptance/settings/update.feature @@ -7,6 +7,10 @@ Feature: settings / update: Update Settings Given 1 datacenter model with the value "datacenter" When I visit the settings page Then the url should be /settings + Then I have settings like yaml + --- + token: ~ + --- And I submit Then I have settings like yaml --- diff --git a/ui-v2/tests/acceptance/steps/dc/acls/use-steps.js b/ui-v2/tests/acceptance/steps/dc/acls/use-steps.js new file mode 100644 index 0000000000..a7eff3228b --- /dev/null +++ b/ui-v2/tests/acceptance/steps/dc/acls/use-steps.js @@ -0,0 +1,10 @@ +import steps from '../../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} diff --git a/ui-v2/tests/pages.js b/ui-v2/tests/pages.js index 175a4dd1c7..f72772b223 100644 --- a/ui-v2/tests/pages.js +++ b/ui-v2/tests/pages.js @@ -35,7 +35,7 @@ export default { kvs: create(kvs(visitable, deletable, clickable, attribute, collection)), kv: create(kv(visitable, submitable, deletable)), acls: create(acls(visitable, deletable, clickable, attribute, collection, aclFilter)), - acl: create(acl(visitable, submitable, deletable)), + acl: create(acl(visitable, submitable, deletable, clickable)), intentions: create( intentions(visitable, deletable, clickable, attribute, collection, intentionFilter) ), diff --git a/ui-v2/tests/pages/dc/acls/edit.js b/ui-v2/tests/pages/dc/acls/edit.js index 1164d4c0ce..53ede15259 100644 --- a/ui-v2/tests/pages/dc/acls/edit.js +++ b/ui-v2/tests/pages/dc/acls/edit.js @@ -1,7 +1,9 @@ -export default function(visitable, submitable, deletable, triggerable) { +export default function(visitable, submitable, deletable, clickable) { return submitable( deletable({ visit: visitable(['/:dc/acls/:acl', '/:dc/acls/create']), + use: clickable('[data-test-use]'), + confirmUse: clickable('button.type-delete'), }) ); } diff --git a/ui-v2/tests/pages/dc/acls/index.js b/ui-v2/tests/pages/dc/acls/index.js index 7c567840e1..d96d935478 100644 --- a/ui-v2/tests/pages/dc/acls/index.js +++ b/ui-v2/tests/pages/dc/acls/index.js @@ -7,6 +7,8 @@ export default function(visitable, deletable, clickable, attribute, collection, name: attribute('data-test-acl', '[data-test-acl]'), acl: clickable('a'), actions: clickable('label'), + use: clickable('[data-test-use]'), + confirmUse: clickable('button.type-delete'), }) ), filter: filter, From f85369c375d527f4b24a6efe618c84f3c74e09f7 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 17:23:33 +0100 Subject: [PATCH 32/54] Add some navigation testing for back buttons and create buttons --- ui-v2/app/templates/dc/acls/edit.hbs | 2 +- ui-v2/app/templates/dc/acls/index.hbs | 2 +- ui-v2/app/templates/dc/intentions/edit.hbs | 2 +- ui-v2/app/templates/dc/intentions/index.hbs | 2 +- ui-v2/app/templates/dc/kv/edit.hbs | 2 +- ui-v2/app/templates/dc/kv/index.hbs | 4 +- ui-v2/app/templates/dc/nodes/show.hbs | 2 +- ui-v2/app/templates/dc/services/show.hbs | 2 +- .../tests/acceptance/page-navigation.feature | 46 +++++++++++++------ .../tests/lib/page-object/createCreatable.js | 11 +++++ ui-v2/tests/pages.js | 8 ++-- ui-v2/tests/pages/dc/acls/index.js | 6 +-- ui-v2/tests/pages/dc/intentions/index.js | 6 +-- ui-v2/tests/pages/dc/kv/index.js | 6 +-- 14 files changed, 67 insertions(+), 34 deletions(-) create mode 100644 ui-v2/tests/lib/page-object/createCreatable.js diff --git a/ui-v2/app/templates/dc/acls/edit.hbs b/ui-v2/app/templates/dc/acls/edit.hbs index e63de26726..4a6f703016 100644 --- a/ui-v2/app/templates/dc/acls/edit.hbs +++ b/ui-v2/app/templates/dc/acls/edit.hbs @@ -1,7 +1,7 @@ {{#app-view class="acl edit" loading=isLoading}} {{#block-slot 'breadcrumbs'}}

      -
    1. All Tokens
      2. +
    2. All Tokens
    {{/block-slot}} {{#block-slot 'header'}} diff --git a/ui-v2/app/templates/dc/acls/index.hbs b/ui-v2/app/templates/dc/acls/index.hbs index 1ca1224ed0..7587d4f2ca 100644 --- a/ui-v2/app/templates/dc/acls/index.hbs +++ b/ui-v2/app/templates/dc/acls/index.hbs @@ -5,7 +5,7 @@ {{/block-slot}} {{#block-slot 'actions'}} - Create + Create {{/block-slot}} {{#block-slot 'toolbar'}} {{#if (gt items.length 0) }} diff --git a/ui-v2/app/templates/dc/intentions/edit.hbs b/ui-v2/app/templates/dc/intentions/edit.hbs index fa90174940..3fc2ba45d1 100644 --- a/ui-v2/app/templates/dc/intentions/edit.hbs +++ b/ui-v2/app/templates/dc/intentions/edit.hbs @@ -1,7 +1,7 @@ {{#app-view class="acl edit" loading=isLoading}} {{#block-slot 'breadcrumbs'}}
      -
    1. All Intentions
      2. +
    2. All Intentions
    {{/block-slot}} {{#block-slot 'header'}} diff --git a/ui-v2/app/templates/dc/intentions/index.hbs b/ui-v2/app/templates/dc/intentions/index.hbs index edbb49d6f9..dfde3056f7 100644 --- a/ui-v2/app/templates/dc/intentions/index.hbs +++ b/ui-v2/app/templates/dc/intentions/index.hbs @@ -5,7 +5,7 @@ {{/block-slot}} {{#block-slot 'actions'}} - Create + Create {{/block-slot}} {{#block-slot 'toolbar'}} {{#if (gt items.length 0) }} diff --git a/ui-v2/app/templates/dc/kv/edit.hbs b/ui-v2/app/templates/dc/kv/edit.hbs index c156c1849d..22c7b39734 100644 --- a/ui-v2/app/templates/dc/kv/edit.hbs +++ b/ui-v2/app/templates/dc/kv/edit.hbs @@ -1,7 +1,7 @@ {{#app-view class="kv edit" loading=isLoading}} {{#block-slot 'breadcrumbs'}}
      -
    1. Key / Values
      2. +
    2. Key / Values
      3. {{#if (not-eq parent.Key '/') }} {{#each (slice 0 -1 (split parent.Key '/')) as |breadcrumb index|}}
    3. {{breadcrumb}}
      4. diff --git a/ui-v2/app/templates/dc/kv/index.hbs b/ui-v2/app/templates/dc/kv/index.hbs index a7822eda65..0733bef0cb 100644 --- a/ui-v2/app/templates/dc/kv/index.hbs +++ b/ui-v2/app/templates/dc/kv/index.hbs @@ -27,9 +27,9 @@ {{/block-slot}} {{#block-slot 'actions'}} {{#if (not-eq parent.Key '/') }} - Create + Create {{else}} - Create + Create {{/if}} {{/block-slot}} {{#block-slot 'content'}} diff --git a/ui-v2/app/templates/dc/nodes/show.hbs b/ui-v2/app/templates/dc/nodes/show.hbs index 78586c65ee..ee143d8822 100644 --- a/ui-v2/app/templates/dc/nodes/show.hbs +++ b/ui-v2/app/templates/dc/nodes/show.hbs @@ -1,7 +1,7 @@ {{#app-view class="node show"}} {{#block-slot 'breadcrumbs'}}
        -
      1. All Nodes
        2. +
      2. All Nodes
      {{/block-slot}} {{#block-slot 'header'}} diff --git a/ui-v2/app/templates/dc/services/show.hbs b/ui-v2/app/templates/dc/services/show.hbs index 0836ccd9a0..d2163548a6 100644 --- a/ui-v2/app/templates/dc/services/show.hbs +++ b/ui-v2/app/templates/dc/services/show.hbs @@ -1,7 +1,7 @@ {{#app-view class="service show"}} {{#block-slot 'breadcrumbs'}}
        -
      1. All Services
        2. +
      2. All Services
      {{/block-slot}} {{#block-slot 'header'}} diff --git a/ui-v2/tests/acceptance/page-navigation.feature b/ui-v2/tests/acceptance/page-navigation.feature index 73d83beb18..1ec219f272 100644 --- a/ui-v2/tests/acceptance/page-navigation.feature +++ b/ui-v2/tests/acceptance/page-navigation.feature @@ -8,38 +8,58 @@ Feature: Page Navigation dc: dc-1 --- Then the url should be /dc-1/services - Scenario: Clicking [Link] in the navigation takes me to [Url] + Scenario: Clicking [Link] in the navigation takes me to [URL] When I visit the services page for yaml --- dc: dc-1 --- When I click [Link] on the navigation - Then the url should be [Url] + Then the url should be [URL] Where: ---------------------------------------- - | Link | Url | + | Link | URL | | nodes | /dc-1/nodes | | kvs | /dc-1/kv | | acls | /dc-1/acls | | intentions | /dc-1/intentions | | settings | /settings | ---------------------------------------- - Scenario: Clicking a [Item] in the [Model] listing + Scenario: Clicking a [Item] in the [Model] listing and back again When I visit the [Model] page for yaml --- dc: dc-1 --- When I click [Item] on the [Model] - Then the url should be [Url] + Then the url should be [URL] + # This should be a page object function + And I click "[data-test-back]" + Then the url should be [Back] Where: - ------------------------------------------------------------------------------------- - | Item | Model | Url | - | service | services | /dc-1/services/service-0 | - | node | nodes | /dc-1/nodes/node-0 | - | kv | kvs | /dc-1/kv/necessitatibus-0/edit | - | intention | intentions | /dc-1/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca | - | acl | acls | /dc-1/acls/anonymous | - ------------------------------------------------------------------------------------- + -------------------------------------------------------------------------------------------------------- + | Item | Model | URL | Back | + | service | services | /dc-1/services/service-0 | /dc-1/services | + | node | nodes | /dc-1/nodes/node-0 | /dc-1/nodes | + | kv | kvs | /dc-1/kv/necessitatibus-0/edit | /dc-1/kv | + | acl | acls | /dc-1/acls/anonymous | /dc-1/acls | + | intention | intentions | /dc-1/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca | /dc-1/intentions | + -------------------------------------------------------------------------------------------------------- @ignore Scenario: Clicking items in the listings, without depending on the salt ^ Then ok + Scenario: Clicking create in the [Model] listing + When I visit the [Model] page for yaml + --- + dc: dc-1 + --- + When I click create + Then the url should be [URL] + # This should be a page object function + And I click "[data-test-back]" + Then the url should be [Back] + Where: + ------------------------------------------------------------------------ + | Item | Model | URL | Back | + | kv | kvs | /dc-1/kv/create | /dc-1/kv | + | acl | acls | /dc-1/acls/create | /dc-1/acls | + | intention | intentions | /dc-1/intentions/create | /dc-1/intentions | + ------------------------------------------------------------------------ diff --git a/ui-v2/tests/lib/page-object/createCreatable.js b/ui-v2/tests/lib/page-object/createCreatable.js new file mode 100644 index 0000000000..55504ea613 --- /dev/null +++ b/ui-v2/tests/lib/page-object/createCreatable.js @@ -0,0 +1,11 @@ +export default function(clickable, is) { + return function(obj) { + return { + ...obj, + ...{ + create: clickable('[data-test-create]'), + createIsEnabled: is(':not(:disabled)', '[data-test-create]'), + }, + }; + }; +} diff --git a/ui-v2/tests/pages.js b/ui-v2/tests/pages.js index f72772b223..1922588f47 100644 --- a/ui-v2/tests/pages.js +++ b/ui-v2/tests/pages.js @@ -2,6 +2,7 @@ import { create, clickable, is, attribute, collection, text } from 'ember-cli-pa import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; import createDeletable from 'consul-ui/tests/lib/page-object/createDeletable'; import createSubmitable from 'consul-ui/tests/lib/page-object/createSubmitable'; +import createCreatable from 'consul-ui/tests/lib/page-object/createCreatable'; import page from 'consul-ui/tests/pages/components/page'; import radiogroup from 'consul-ui/tests/lib/page-object/radiogroup'; @@ -25,6 +26,7 @@ import intention from 'consul-ui/tests/pages/dc/intentions/edit'; const deletable = createDeletable(clickable); const submitable = createSubmitable(clickable, is); +const creatable = createCreatable(clickable, is); export default { index: create(index(visitable, collection)), dcs: create(dcs(visitable, clickable, attribute, collection)), @@ -32,12 +34,12 @@ export default { service: create(service(visitable, attribute, collection, text, catalogFilter)), nodes: create(nodes(visitable, clickable, attribute, collection, catalogFilter)), node: create(node(visitable, deletable, clickable, attribute, collection, radiogroup)), - kvs: create(kvs(visitable, deletable, clickable, attribute, collection)), + kvs: create(kvs(visitable, deletable, creatable, clickable, attribute, collection)), kv: create(kv(visitable, submitable, deletable)), - acls: create(acls(visitable, deletable, clickable, attribute, collection, aclFilter)), + acls: create(acls(visitable, deletable, creatable, clickable, attribute, collection, aclFilter)), acl: create(acl(visitable, submitable, deletable, clickable)), intentions: create( - intentions(visitable, deletable, clickable, attribute, collection, intentionFilter) + intentions(visitable, deletable, creatable, clickable, attribute, collection, intentionFilter) ), intention: create(intention(visitable, submitable, deletable)), settings: create(settings(visitable, submitable)), diff --git a/ui-v2/tests/pages/dc/acls/index.js b/ui-v2/tests/pages/dc/acls/index.js index d96d935478..f8b6b53fad 100644 --- a/ui-v2/tests/pages/dc/acls/index.js +++ b/ui-v2/tests/pages/dc/acls/index.js @@ -1,5 +1,5 @@ -export default function(visitable, deletable, clickable, attribute, collection, filter) { - return { +export default function(visitable, deletable, creatable, clickable, attribute, collection, filter) { + return creatable({ visit: visitable('/:dc/acls'), acls: collection( '[data-test-tabular-row]', @@ -12,5 +12,5 @@ export default function(visitable, deletable, clickable, attribute, collection, }) ), filter: filter, - }; + }); } diff --git a/ui-v2/tests/pages/dc/intentions/index.js b/ui-v2/tests/pages/dc/intentions/index.js index 5ab14b01b0..5bd21010b5 100644 --- a/ui-v2/tests/pages/dc/intentions/index.js +++ b/ui-v2/tests/pages/dc/intentions/index.js @@ -1,5 +1,5 @@ -export default function(visitable, deletable, clickable, attribute, collection, filter) { - return { +export default function(visitable, deletable, creatable, clickable, attribute, collection, filter) { + return creatable({ visit: visitable('/:dc/intentions'), intentions: collection( '[data-test-tabular-row]', @@ -15,5 +15,5 @@ export default function(visitable, deletable, clickable, attribute, collection, }) ), filter: filter, - }; + }); } diff --git a/ui-v2/tests/pages/dc/kv/index.js b/ui-v2/tests/pages/dc/kv/index.js index a0c8439145..27c3dda55e 100644 --- a/ui-v2/tests/pages/dc/kv/index.js +++ b/ui-v2/tests/pages/dc/kv/index.js @@ -1,5 +1,5 @@ -export default function(visitable, deletable, clickable, attribute, collection) { - return { +export default function(visitable, deletable, creatable, clickable, attribute, collection) { + return creatable({ visit: visitable('/:dc/kv'), kvs: collection( '[data-test-tabular-row]', @@ -9,5 +9,5 @@ export default function(visitable, deletable, clickable, attribute, collection) actions: clickable('label'), }) ), - }; + }); } From 15b627a5170a09ab64be82b17a307cc1753760e9 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 18:39:15 +0100 Subject: [PATCH 33/54] Basic acceptance testing for navigating via cancel buttons --- .../tests/acceptance/page-navigation.feature | 20 +++++++++++++++++-- .../tests/lib/page-object/createCancelable.js | 11 ++++++++++ ui-v2/tests/pages.js | 8 +++++--- ui-v2/tests/pages/dc/acls/edit.js | 14 +++++++------ ui-v2/tests/pages/dc/intentions/edit.js | 10 ++++++---- ui-v2/tests/pages/dc/kv/edit.js | 10 ++++++---- 6 files changed, 54 insertions(+), 19 deletions(-) create mode 100644 ui-v2/tests/lib/page-object/createCancelable.js diff --git a/ui-v2/tests/acceptance/page-navigation.feature b/ui-v2/tests/acceptance/page-navigation.feature index 1ec219f272..95a88fa044 100644 --- a/ui-v2/tests/acceptance/page-navigation.feature +++ b/ui-v2/tests/acceptance/page-navigation.feature @@ -31,7 +31,6 @@ Feature: Page Navigation --- When I click [Item] on the [Model] Then the url should be [URL] - # This should be a page object function And I click "[data-test-back]" Then the url should be [Back] Where: @@ -43,6 +42,22 @@ Feature: Page Navigation | acl | acls | /dc-1/acls/anonymous | /dc-1/acls | | intention | intentions | /dc-1/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca | /dc-1/intentions | -------------------------------------------------------------------------------------------------------- + Scenario: Clicking a [Item] in the [Model] listing and canceling + When I visit the [Model] page for yaml + --- + dc: dc-1 + --- + When I click [Item] on the [Model] + Then the url should be [URL] + And I click "[type=reset]" + Then the url should be [Back] + Where: + -------------------------------------------------------------------------------------------------------- + | Item | Model | URL | Back | + | kv | kvs | /dc-1/kv/necessitatibus-0/edit | /dc-1/kv | + | acl | acls | /dc-1/acls/anonymous | /dc-1/acls | + | intention | intentions | /dc-1/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca | /dc-1/intentions | + -------------------------------------------------------------------------------------------------------- @ignore Scenario: Clicking items in the listings, without depending on the salt ^ Then ok @@ -53,7 +68,6 @@ Feature: Page Navigation --- When I click create Then the url should be [URL] - # This should be a page object function And I click "[data-test-back]" Then the url should be [Back] Where: @@ -63,3 +77,5 @@ Feature: Page Navigation | acl | acls | /dc-1/acls/create | /dc-1/acls | | intention | intentions | /dc-1/intentions/create | /dc-1/intentions | ------------------------------------------------------------------------ + Scenario: Using I click on should change the currentPage ^ + Then ok diff --git a/ui-v2/tests/lib/page-object/createCancelable.js b/ui-v2/tests/lib/page-object/createCancelable.js new file mode 100644 index 0000000000..390362e86d --- /dev/null +++ b/ui-v2/tests/lib/page-object/createCancelable.js @@ -0,0 +1,11 @@ +export default function(clickable, is) { + return function(obj) { + return { + ...obj, + ...{ + cancel: clickable('[type=reset]'), + cancelIsEnabled: is(':not(:disabled)', '[type=reset]'), + }, + }; + }; +} diff --git a/ui-v2/tests/pages.js b/ui-v2/tests/pages.js index 1922588f47..6720acc544 100644 --- a/ui-v2/tests/pages.js +++ b/ui-v2/tests/pages.js @@ -3,6 +3,7 @@ import { visitable } from 'consul-ui/tests/lib/page-object/visitable'; import createDeletable from 'consul-ui/tests/lib/page-object/createDeletable'; import createSubmitable from 'consul-ui/tests/lib/page-object/createSubmitable'; import createCreatable from 'consul-ui/tests/lib/page-object/createCreatable'; +import createCancelable from 'consul-ui/tests/lib/page-object/createCancelable'; import page from 'consul-ui/tests/pages/components/page'; import radiogroup from 'consul-ui/tests/lib/page-object/radiogroup'; @@ -27,6 +28,7 @@ import intention from 'consul-ui/tests/pages/dc/intentions/edit'; const deletable = createDeletable(clickable); const submitable = createSubmitable(clickable, is); const creatable = createCreatable(clickable, is); +const cancelable = createCancelable(clickable, is); export default { index: create(index(visitable, collection)), dcs: create(dcs(visitable, clickable, attribute, collection)), @@ -35,12 +37,12 @@ export default { nodes: create(nodes(visitable, clickable, attribute, collection, catalogFilter)), node: create(node(visitable, deletable, clickable, attribute, collection, radiogroup)), kvs: create(kvs(visitable, deletable, creatable, clickable, attribute, collection)), - kv: create(kv(visitable, submitable, deletable)), + kv: create(kv(visitable, submitable, deletable, cancelable, clickable)), acls: create(acls(visitable, deletable, creatable, clickable, attribute, collection, aclFilter)), - acl: create(acl(visitable, submitable, deletable, clickable)), + acl: create(acl(visitable, submitable, deletable, cancelable, clickable)), intentions: create( intentions(visitable, deletable, creatable, clickable, attribute, collection, intentionFilter) ), - intention: create(intention(visitable, submitable, deletable)), + intention: create(intention(visitable, submitable, deletable, cancelable)), settings: create(settings(visitable, submitable)), }; diff --git a/ui-v2/tests/pages/dc/acls/edit.js b/ui-v2/tests/pages/dc/acls/edit.js index 53ede15259..71c0620717 100644 --- a/ui-v2/tests/pages/dc/acls/edit.js +++ b/ui-v2/tests/pages/dc/acls/edit.js @@ -1,9 +1,11 @@ -export default function(visitable, submitable, deletable, clickable) { +export default function(visitable, submitable, deletable, cancelable, clickable) { return submitable( - deletable({ - visit: visitable(['/:dc/acls/:acl', '/:dc/acls/create']), - use: clickable('[data-test-use]'), - confirmUse: clickable('button.type-delete'), - }) + cancelable( + deletable({ + visit: visitable(['/:dc/acls/:acl', '/:dc/acls/create']), + use: clickable('[data-test-use]'), + confirmUse: clickable('button.type-delete'), + }) + ) ); } diff --git a/ui-v2/tests/pages/dc/intentions/edit.js b/ui-v2/tests/pages/dc/intentions/edit.js index 5c6d94550a..a9cdff3866 100644 --- a/ui-v2/tests/pages/dc/intentions/edit.js +++ b/ui-v2/tests/pages/dc/intentions/edit.js @@ -1,7 +1,9 @@ -export default function(visitable, submitable, deletable) { +export default function(visitable, submitable, deletable, cancelable) { return submitable( - deletable({ - visit: visitable(['/:dc/intentions/:intention', '/:dc/intentions/create']), - }) + cancelable( + deletable({ + visit: visitable(['/:dc/intentions/:intention', '/:dc/intentions/create']), + }) + ) ); } diff --git a/ui-v2/tests/pages/dc/kv/edit.js b/ui-v2/tests/pages/dc/kv/edit.js index f7a019705b..b644876cc0 100644 --- a/ui-v2/tests/pages/dc/kv/edit.js +++ b/ui-v2/tests/pages/dc/kv/edit.js @@ -1,7 +1,9 @@ -export default function(visitable, submitable, deletable) { +export default function(visitable, submitable, deletable, cancelable) { return submitable( - deletable({ - visit: visitable(['/:dc/kv/:kv/edit', '/:dc/kv/create'], str => str), - }) + cancelable( + deletable({ + visit: visitable(['/:dc/kv/:kv/edit', '/:dc/kv/create'], str => str), + }) + ) ); } From 7b8b3a012cc7782aeb7efdb2e831a84a5e38dc77 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 4 Jul 2018 18:53:52 +0100 Subject: [PATCH 34/54] Upgrade consul-api-double for session destroy --- ui-v2/yarn.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui-v2/yarn.lock b/ui-v2/yarn.lock index 50303e742d..d575cd75b2 100644 --- a/ui-v2/yarn.lock +++ b/ui-v2/yarn.lock @@ -82,8 +82,8 @@ js-yaml "^3.10.0" "@hashicorp/consul-api-double@^1.2.0": - version "1.2.0" - resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-1.2.0.tgz#2cd2a991818e13e7b97803af3d62ec6c9cb83b28" + version "1.3.0" + resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-1.3.0.tgz#fded48ca4db1e63c66e39b4433b2169b6add69ed" "@hashicorp/ember-cli-api-double@^1.3.0": version "1.3.0" From 83caa6a2966bc38fdd8f2c78e331977360db3ae1 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Thu, 5 Jul 2018 09:20:58 +0100 Subject: [PATCH 35/54] Add some more detail to the README pre-adding a CONTRIBUTING --- ui-v2/README.md | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/ui-v2/README.md b/ui-v2/README.md index 82092f19a6..9e6da2e0d3 100644 --- a/ui-v2/README.md +++ b/ui-v2/README.md @@ -13,37 +13,27 @@ You will need the following things properly installed on your computer. ## Installation -* `git clone ` this repository -* `cd ui` +* `git clone https://github.com/hashicorp/consul.git` this repository +* `cd ui-v2` * `yarn install` ## Running / Development -* `yarn run start` +* `make start-api` or `yarn start:api` (this starts a Consul API double running +on http://localhost:3000) +* `make start` or `yarn start` to start the ember app that connects to the +above API double * Visit your app at [http://localhost:4200](http://localhost:4200). * Visit your tests at [http://localhost:4200/tests](http://localhost:4200/tests). + ### Code Generators Make use of the many generators for code, try `ember help generate` for more details ### Running Tests -* `ember test` -* `ember test --server` +You do not need to run `make start-api`/`yarn run start:api` to run the tests -### Building - -* `ember build` (development) -* `ember build --environment production` (production) - -### Deploying - - -## Further Reading / Useful Links - -* [ember.js](https://emberjs.com/) -* [ember-cli](https://ember-cli.com/) -* Development Browser Extensions - * [ember inspector for chrome](https://chrome.google.com/webstore/detail/ember-inspector/bmdblncegkenkacieihfhpjfppoconhi) - * [ember inspector for firefox](https://addons.mozilla.org/en-US/firefox/addon/ember-inspector/) +* `make test` or `yarn run test` +* `make test-view` or `yarn run test:view` to view the tests running in Chrome From 6a407a044e48c6c83fa7572d5c23c46b09738d0f Mon Sep 17 00:00:00 2001 From: John Cowen Date: Thu, 5 Jul 2018 13:33:02 +0100 Subject: [PATCH 36/54] Remove validation for presence of KV values --- ui-v2/app/validations/kv.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui-v2/app/validations/kv.js b/ui-v2/app/validations/kv.js index 859359c37a..17d4f1b1c0 100644 --- a/ui-v2/app/validations/kv.js +++ b/ui-v2/app/validations/kv.js @@ -1,5 +1,4 @@ import { validatePresence, validateLength } from 'ember-changeset-validations/validators'; export default { Key: [validatePresence(true), validateLength({ min: 1 })], - Value: validatePresence(true), }; From b29546e578baf4dcce63fa226e1e66dd7f9e6e4d Mon Sep 17 00:00:00 2001 From: John Cowen Date: Thu, 5 Jul 2018 13:35:06 +0100 Subject: [PATCH 37/54] Looking into atob functionality, consequence of Value: null The Consul API can pass through `Value: null` which does not get cast to a string by ember-data. This snowballs into problems with `atob` which then tried to decode `null`. There are 2 problems here. 1. `Value` should never be `null` - I've added a removeNull function to shallowly loop though props and remove properties that are `null`, for the moment this is only on single KV JSON responses - therefore `Value` will never be `null` which is the root of the problem 2. `atob` doesn't quite follow the `window.atob` API in that the `window.atob` API casts everything down to a string first, therefore it will try to decode `null` > `'null'` > `crazy unicode thing`. - I've commented in a fix for this, but whilst this shouldn't be causing anymore problems in our UI (now that `Value` is never `null`), I'll uncomment it in another future release. Tests are already written for it which more closely follow `window.atob` but skipped for now (next commit) --- ui-v2/app/adapters/kv.js | 3 +- ui-v2/app/utils/atob.js | 1 + ui-v2/app/utils/remove-null.js | 9 +++ ui-v2/tests/helpers/stub-super.js | 1 + ui-v2/tests/unit/adapters/kv-test.js | 2 +- ui-v2/tests/unit/utils/atob-test.js | 67 ++++++++++++++++++++++ ui-v2/tests/unit/utils/remove-null-test.js | 49 ++++++++++++++++ 7 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 ui-v2/app/utils/remove-null.js create mode 100644 ui-v2/tests/unit/utils/atob-test.js create mode 100644 ui-v2/tests/unit/utils/remove-null-test.js diff --git a/ui-v2/app/adapters/kv.js b/ui-v2/app/adapters/kv.js index 309d467889..4f21239b7c 100644 --- a/ui-v2/app/adapters/kv.js +++ b/ui-v2/app/adapters/kv.js @@ -11,6 +11,7 @@ import { get } from '@ember/object'; import { inject as service } from '@ember/service'; import keyToArray from 'consul-ui/utils/keyToArray'; +import removeNull from 'consul-ui/utils/remove-null'; import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/kv'; import { FOREIGN_KEY as DATACENTER_KEY } from 'consul-ui/models/dc'; @@ -98,7 +99,7 @@ export default Adapter.extend({ break; case this.isQueryRecord(url): response = { - ...response[0], + ...removeNull(response[0]), ...{ [PRIMARY_KEY]: this.uidForURL(url), }, diff --git a/ui-v2/app/utils/atob.js b/ui-v2/app/utils/atob.js index edac64c916..237a4b5d4a 100644 --- a/ui-v2/app/utils/atob.js +++ b/ui-v2/app/utils/atob.js @@ -1,6 +1,7 @@ import TextEncoderLite from 'npm:text-encoder-lite'; import base64js from 'npm:base64-js'; export default function(str, encoding = 'utf-8') { + // str = String(str).trim(); //decode const bytes = base64js.toByteArray(str); return new (TextDecoder || TextEncoderLite)(encoding).decode(bytes); diff --git a/ui-v2/app/utils/remove-null.js b/ui-v2/app/utils/remove-null.js new file mode 100644 index 0000000000..c624365ea9 --- /dev/null +++ b/ui-v2/app/utils/remove-null.js @@ -0,0 +1,9 @@ +export default function(obj) { + // non-recursive for the moment + return Object.keys(obj).reduce(function(prev, item, i, arr) { + if (obj[item] !== null) { + return { ...prev, ...{ [item]: obj[item] } }; + } + return prev; + }, {}); +} diff --git a/ui-v2/tests/helpers/stub-super.js b/ui-v2/tests/helpers/stub-super.js index af19a48164..a10a2a37cf 100644 --- a/ui-v2/tests/helpers/stub-super.js +++ b/ui-v2/tests/helpers/stub-super.js @@ -38,6 +38,7 @@ export default function(obj, stub) { return _super; }, }); + // TODO: try/catch this? const actual = cb(); Object.defineProperty(Object.getPrototypeOf(obj), '_super', { set: function(val) { diff --git a/ui-v2/tests/unit/adapters/kv-test.js b/ui-v2/tests/unit/adapters/kv-test.js index c0f03b1746..a1bcb35556 100644 --- a/ui-v2/tests/unit/adapters/kv-test.js +++ b/ui-v2/tests/unit/adapters/kv-test.js @@ -46,7 +46,7 @@ module('Unit | Adapter | kv', function(hooks) { const uid = { uid: JSON.stringify([dc, expected]), }; - const actual = adapter.handleResponse(200, {}, uid, { url: url }); + const actual = adapter.handleResponse(200, {}, [uid], { url: url }); assert.deepEqual(actual, uid); }); }); diff --git a/ui-v2/tests/unit/utils/atob-test.js b/ui-v2/tests/unit/utils/atob-test.js new file mode 100644 index 0000000000..a0d8ce8952 --- /dev/null +++ b/ui-v2/tests/unit/utils/atob-test.js @@ -0,0 +1,67 @@ +import { module } from 'ember-qunit'; +import test from 'ember-sinon-qunit/test-support/test'; +import { skip } from 'qunit'; +import atob from 'consul-ui/utils/atob'; +module('Unit | Utils | atob', {}); + +skip('it decodes non-strings properly', function(assert) { + [ + { + test: ' ', + expected: '', + }, + { + test: new String(), + expected: '', + }, + { + test: new String('MTIzNA=='), + expected: '1234', + }, + { + test: [], + expected: '', + }, + { + test: [' '], + expected: '', + }, + { + test: new Array(), + expected: '', + }, + { + test: ['MTIzNA=='], + expected: '1234', + }, + { + test: null, + expected: '��e', + }, + ].forEach(function(item) { + const actual = atob(item.test); + assert.equal(actual, item.expected); + }); +}); +test('it decodes strings properly', function(assert) { + [ + { + test: '', + expected: '', + }, + { + test: 'MTIzNA==', + expected: '1234', + }, + ].forEach(function(item) { + const actual = atob(item.test); + assert.equal(actual, item.expected); + }); +}); +test('throws when passed the wrong value', function(assert) { + [{}, ['MTIz', 'NA=='], new Number(), 'hi'].forEach(function(item) { + assert.throws(function() { + atob(item); + }); + }); +}); diff --git a/ui-v2/tests/unit/utils/remove-null-test.js b/ui-v2/tests/unit/utils/remove-null-test.js new file mode 100644 index 0000000000..c887deb595 --- /dev/null +++ b/ui-v2/tests/unit/utils/remove-null-test.js @@ -0,0 +1,49 @@ +import removeNull from 'consul-ui/utils/remove-null'; +import { skip } from 'qunit'; +import { module, test } from 'qunit'; + +module('Unit | Utility | remove null'); + +test('it removes null valued properties shallowly', function(assert) { + [ + { + test: { + Value: null, + }, + expected: {}, + }, + { + test: { + Key: 'keyname', + Value: null, + }, + expected: { + Key: 'keyname', + }, + }, + { + test: { + Key: 'keyname', + Value: '', + }, + expected: { + Key: 'keyname', + Value: '', + }, + }, + { + test: { + Key: 'keyname', + Value: false, + }, + expected: { + Key: 'keyname', + Value: false, + }, + }, + ].forEach(function(item) { + const actual = removeNull(item.test); + assert.deepEqual(actual, item.expected); + }); +}); +skip('it removes null valued properties deeply'); From 67402b3d26dc4f59b45408d4d29894ca4f73622a Mon Sep 17 00:00:00 2001 From: John Cowen Date: Thu, 5 Jul 2018 13:43:03 +0100 Subject: [PATCH 38/54] Tests and comments regarding the previous 2 commits --- ui-v2/app/models/kv.js | 6 ++- ui-v2/tests/acceptance/dc/kvs/update.feature | 51 +++++++++++++++++++- ui-v2/tests/helpers/type-to-url.js | 26 +++++----- ui-v2/tests/steps.js | 18 +++++-- ui-v2/yarn.lock | 4 +- 5 files changed, 82 insertions(+), 23 deletions(-) diff --git a/ui-v2/app/models/kv.js b/ui-v2/app/models/kv.js index 66e3a2126c..313cb587a3 100644 --- a/ui-v2/app/models/kv.js +++ b/ui-v2/app/models/kv.js @@ -13,7 +13,11 @@ export default Model.extend({ [SLUG_KEY]: attr('string'), LockIndex: attr('number'), Flags: attr('number'), - Value: attr('string'), + // TODO: Consider defaulting all strings to '' because `typeof null !== 'string'` + // look into what other transformers do with `null` also + // preferably removeNull would be done in this layer also as if a property is `null` + // default Values don't kick in, which also explains `Tags` elsewhere + Value: attr('string'), //, {defaultValue: function() {return '';}} CreateIndex: attr('string'), ModifyIndex: attr('string'), Session: attr('string'), diff --git a/ui-v2/tests/acceptance/dc/kvs/update.feature b/ui-v2/tests/acceptance/dc/kvs/update.feature index 1f00f7c257..ba54ebf6c9 100644 --- a/ui-v2/tests/acceptance/dc/kvs/update.feature +++ b/ui-v2/tests/acceptance/dc/kvs/update.feature @@ -1,7 +1,8 @@ @setupApplicationTest Feature: dc / kvs / update: KV Update - Scenario: Update to [Name] change value to [Value] + Background: Given 1 datacenter model with the value "datacenter" + Scenario: Update to [Name] change value to [Value] And 1 kv model from yaml --- Key: [Name] @@ -25,6 +26,54 @@ Feature: dc / kvs / update: KV Update | key-name | a value | | folder/key-name | a value | -------------------------------------------- + Scenario: Update to a key change value to ' ' + And 1 kv model from yaml + --- + Key: key + --- + When I visit the kv page for yaml + --- + dc: datacenter + kv: key + --- + Then the url should be /datacenter/kv/key/edit + Then I fill in with yaml + --- + value: ' ' + --- + And I submit + Then a PUT request is made to "/v1/kv/key?dc=datacenter" with the body " " + Scenario: Update to a key change value to '' + And 1 kv model from yaml + --- + Key: key + --- + When I visit the kv page for yaml + --- + dc: datacenter + kv: key + --- + Then the url should be /datacenter/kv/key/edit + Then I fill in with yaml + --- + value: '' + --- + And I submit + Then a PUT request is made to "/v1/kv/key?dc=datacenter" with no body + Scenario: Update to a key when the value is empty + And 1 kv model from yaml + --- + Key: key + Value: ~ + --- + When I visit the kv page for yaml + --- + dc: datacenter + kv: key + --- + Then the url should be /datacenter/kv/key/edit + And I submit + Then a PUT request is made to "/v1/kv/key?dc=datacenter" with no body @ignore Scenario: The feedback dialog says success or failure Then ok diff --git a/ui-v2/tests/helpers/type-to-url.js b/ui-v2/tests/helpers/type-to-url.js index 470a5f2915..401c8c0b91 100644 --- a/ui-v2/tests/helpers/type-to-url.js +++ b/ui-v2/tests/helpers/type-to-url.js @@ -1,34 +1,32 @@ export default function(type) { - let url = null; + let requests = null; switch (type) { case 'dc': - url = ['/v1/catalog/datacenters']; + requests = ['/v1/catalog/datacenters']; break; case 'service': - url = ['/v1/internal/ui/services', '/v1/health/service/']; + requests = ['/v1/internal/ui/services', '/v1/health/service/']; break; case 'node': - url = ['/v1/internal/ui/nodes']; + requests = ['/v1/internal/ui/nodes']; break; case 'kv': - url = '/v1/kv/'; + requests = ['/v1/kv/']; break; case 'acl': - url = ['/v1/acl/list']; + requests = ['/v1/acl/list']; break; case 'session': - url = ['/v1/session/node/']; + requests = ['/v1/session/node/']; break; } - return function(actual) { - if (url === null) { + // TODO: An instance of URL should come in here (instead of 2 args) + return function(url, method) { + if (requests === null) { return false; } - if (typeof url === 'string') { - return url === actual; - } - return url.some(function(item) { - return actual.indexOf(item) === 0; + return requests.some(function(item) { + return method.toUpperCase() === 'GET' && url.indexOf(item) === 0; }); }; } diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index 901843bd12..b25655e4c1 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -207,12 +207,20 @@ export default function(assert) { ); assert.equal(request.url, url, `Expected the request url to be ${url}, was ${request.url}`); const body = request.requestBody; - assert.equal( - body, - data, - `Expected the request body to be ${body}, was ${request.requestBody}` - ); + assert.equal(body, data, `Expected the request body to be ${data}, was ${body}`); }) + .then('a $method request is made to "$url" with no body', function(method, url) { + const request = api.server.history[api.server.history.length - 2]; + assert.equal( + request.method, + method, + `Expected the request method to be ${method}, was ${request.method}` + ); + assert.equal(request.url, url, `Expected the request url to be ${url}, was ${request.url}`); + const body = request.requestBody; + assert.equal(body, null, `Expected the request body to be null, was ${body}`); + }) + .then('a $method request is made to "$url"', function(method, url) { const request = api.server.history[api.server.history.length - 2]; assert.equal( diff --git a/ui-v2/yarn.lock b/ui-v2/yarn.lock index 50303e742d..6b5f7f3443 100644 --- a/ui-v2/yarn.lock +++ b/ui-v2/yarn.lock @@ -70,8 +70,8 @@ "@glimmer/di" "^0.2.0" "@hashicorp/api-double@^1.3.0": - version "1.3.1" - resolved "https://registry.yarnpkg.com/@hashicorp/api-double/-/api-double-1.3.1.tgz#fd9d706674b934857a638459c2bb52d2f2809455" + version "1.4.0" + resolved "https://registry.yarnpkg.com/@hashicorp/api-double/-/api-double-1.4.0.tgz#17ddad8e55370de0d24151a38c5f029bc207cafe" dependencies: "@gardenhq/o" "^8.0.1" "@gardenhq/tick-control" "^2.0.0" From 1cd1b5568284119a34b3e77196c5c14d0aed31aa Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Thu, 5 Jul 2018 22:04:29 -0400 Subject: [PATCH 39/54] Agent/Proxy : Properly passes env variables to child --- agent/proxy/manager.go | 2 +- agent/proxy/manager_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/agent/proxy/manager.go b/agent/proxy/manager.go index e3c6dd7796..e22079b186 100644 --- a/agent/proxy/manager.go +++ b/agent/proxy/manager.go @@ -432,7 +432,7 @@ func (m *Manager) newProxy(mp *local.ManagedProxy) (Proxy, error) { if err := m.configureLogDir(id, &cmd); err != nil { return nil, fmt.Errorf("error configuring proxy logs: %s", err) } - + cmd.Env = os.Environ() // Build the daemon structure proxy.Command = &cmd proxy.ProxyID = id diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 42286493c7..b3269bf10c 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -261,6 +261,46 @@ func TestManagerRun_daemonPid(t *testing.T) { require.NotEmpty(pidRaw) } +// Test to check if the parent and the child processes +// have the same environmental variables + +func TestEnvironproxy(t *testing.T) { + t.Parallel() + + require := require.New(t) + state := local.TestState(t) + m, closer := testManager(t) + defer closer() + m.State = state + defer m.Kill() + + // Add the proxy + td, closer := testTempDir(t) + defer closer() + path := filepath.Join(td, "env-variables") + d := &Daemon{ + Command: helperProcess("parent", path), + Logger: testLogger, + } + var currentEnviron []string + currentEnviron = os.Environ() + // Environmental variable of the helper + if err := d.Start(); err != nil { + t.Error("Daemon failed to start") + defer d.Stop() + envProcess := d.Command.Env + var envData []byte + for _, data := range envProcess { + envData = append(envData, []byte(data)...) + envData = append(envData, []byte("\n")...) + } + t.Logf("Env Data:%s", envProcess) + //Get the parent environmental variables in a file + + require.Equal(currentEnviron, envProcess) + } +} + // Test the Snapshot/Restore works. func TestManagerRun_snapshotRestore(t *testing.T) { t.Parallel() From a5fa27d71809bb182834494aafff81ee3c06e829 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Fri, 6 Jul 2018 11:01:45 +0100 Subject: [PATCH 40/54] Fix a couple of typos in the comments --- ui-v2/tests/acceptance/deleting.feature | 2 +- ui-v2/tests/steps.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui-v2/tests/acceptance/deleting.feature b/ui-v2/tests/acceptance/deleting.feature index 30808f77cd..5f7132881f 100644 --- a/ui-v2/tests/acceptance/deleting.feature +++ b/ui-v2/tests/acceptance/deleting.feature @@ -1,5 +1,5 @@ @setupApplicationTest -Feature: deleting: Deleting form the listing and the detail page with confirmation +Feature: deleting: Deleting from the listing and the detail page with confirmation Scenario: Deleting a [Model] from the [Model] listing page Given 1 datacenter model with the value "datacenter" And 1 [Model] model from json diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index b8d0a732f0..62466ad72c 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -85,7 +85,7 @@ export default function(assert) { .when('I click "$selector"', function(selector) { return click(selector); }) - // TODO: Probably nicer to thing of better vocab than having the 'without " rule' + // TODO: Probably nicer to think of better vocab than having the 'without " rule' .when('I click (?!")$property(?!")', function(property) { try { return currentPage[property](); From c01fb37c46c08411c500e237b4c3f59d3b448133 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Fri, 6 Jul 2018 13:09:23 +0100 Subject: [PATCH 41/54] Don't clone prev, there's no need --- ui-v2/app/utils/remove-null.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui-v2/app/utils/remove-null.js b/ui-v2/app/utils/remove-null.js index c624365ea9..6da1a68b18 100644 --- a/ui-v2/app/utils/remove-null.js +++ b/ui-v2/app/utils/remove-null.js @@ -2,7 +2,7 @@ export default function(obj) { // non-recursive for the moment return Object.keys(obj).reduce(function(prev, item, i, arr) { if (obj[item] !== null) { - return { ...prev, ...{ [item]: obj[item] } }; + prev[item] = obj[item]; } return prev; }, {}); From 5eecbeb7ae837717c4dba38102f4dc62f1e42ae1 Mon Sep 17 00:00:00 2001 From: Geoffrey Grosenbach Date: Thu, 5 Jul 2018 15:23:39 -0700 Subject: [PATCH 42/54] Notes that both "hcl" and "json" files are loaded from the config-dir. The previous version only mentioned "json" but the behavior was to read "hcl" files as well. --- website/source/docs/agent/options.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index 4e2ecb3033..b0d4438c6e 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -136,7 +136,7 @@ will exit with an error at startup. * `-config-dir` - A directory of configuration files to load. Consul will - load all files in this directory with the suffix ".json". The load order + load all files in this directory with the suffix ".json" or ".hcl". The load order is alphabetical, and the the same merge routine is used as with the [`config-file`](#_config_file) option above. This option can be specified multiple times to load multiple directories. Sub-directories of the config directory are not loaded. From 401b206a2e08bf5b8b69272fae4a46a2d9c226d9 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 6 Jul 2018 16:05:25 -0700 Subject: [PATCH 43/54] Store the time CARoot is rotated out instead of when to prune --- agent/consul/connect_ca_endpoint.go | 2 +- agent/consul/leader.go | 16 ++++++---------- agent/structs/connect_ca.go | 6 +++--- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 1e04f6733e..4cdb72ff7c 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -178,7 +178,7 @@ func (s *ConnectCA) ConfigurationSet( newRoot := *r if newRoot.Active { newRoot.Active = false - newRoot.RotateOutAt = time.Now().Add(caRootExpireDuration) + newRoot.RotatedOutAt = time.Now() } newRoots = append(newRoots, &newRoot) } diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 2b0497dc38..8b32226dcb 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -33,7 +33,7 @@ var ( caRootPruneInterval = time.Hour // caRootExpireDuration is the duration after which an inactive root is considered - // "expired". + // "expired". Currently this is based on the default leaf cert TTL of 3 days. caRootExpireDuration = 7 * 24 * time.Hour // minAutopilotVersion is the minimum Consul version in which Autopilot features @@ -568,10 +568,6 @@ func (s *Server) setCAProvider(newProvider ca.Provider, root *structs.CARoot) { // startCARootPruning starts a goroutine that looks for stale CARoots // and removes them from the state store. func (s *Server) startCARootPruning() { - if !s.config.ConnectEnabled { - return - } - s.caPruningLock.Lock() defer s.caPruningLock.Unlock() @@ -602,6 +598,10 @@ func (s *Server) startCARootPruning() { // pruneCARoots looks for any CARoots that have been rotated out and expired. func (s *Server) pruneCARoots() error { + if !s.config.ConnectEnabled { + return nil + } + idx, roots, err := s.fsm.State().CARoots(nil) if err != nil { return err @@ -609,7 +609,7 @@ func (s *Server) pruneCARoots() error { var newRoots structs.CARoots for _, r := range roots { - if !r.Active && !r.RotateOutAt.IsZero() && r.RotateOutAt.Before(time.Now()) { + if !r.Active && !r.RotatedOutAt.IsZero() && time.Now().Sub(r.RotatedOutAt) > caRootExpireDuration { s.logger.Printf("[INFO] connect: pruning old unused root CA (ID: %s)", r.ID) continue } @@ -640,10 +640,6 @@ func (s *Server) pruneCARoots() error { // stopCARootPruning stops the CARoot pruning process. func (s *Server) stopCARootPruning() { - if !s.config.ConnectEnabled { - return - } - s.caPruningLock.Lock() defer s.caPruningLock.Unlock() diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 52b9c16f15..375a7df325 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -73,10 +73,10 @@ type CARoot struct { // cannot be active. Active bool - // RotateOutAt is the time at which this CA can be removed from the state. + // RotatedOutAt is the time at which this CA was removed from the state. // This will only be set on roots that have been rotated out from being the - // active one. - RotateOutAt time.Time `json:"-"` + // active root. + RotatedOutAt time.Time `json:"-"` RaftIndex } From 0e4e451a5656b19a59d2ada4ccdfea2449f0158f Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Sat, 7 Jul 2018 14:03:34 +0200 Subject: [PATCH 44/54] Fixed indentation in test --- agent/acl_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/agent/acl_test.go b/agent/acl_test.go index 0932dc5254..6b912646e3 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -277,9 +277,9 @@ func TestACL_Down_Extend(t *testing.T) { aclExtendPolicies := []string{"extend-cache", "async-cache"} for _, aclDownPolicy := range aclExtendPolicies { a := NewTestAgent(t.Name(), TestACLConfig()+` - acl_down_policy = "`+aclDownPolicy+`" - acl_enforce_version_8 = true - `) + acl_down_policy = "`+aclDownPolicy+`" + acl_enforce_version_8 = true + `) defer a.Shutdown() m := MockServer{ From a937c7fa7073954a4220d168fa2292c757460c3a Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Mon, 9 Jul 2018 11:36:33 +0200 Subject: [PATCH 45/54] Added new ACL blocked Metrics to telemetry.html --- website/source/docs/agent/telemetry.html.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/website/source/docs/agent/telemetry.html.md b/website/source/docs/agent/telemetry.html.md index a0a9685c36..ad268d1588 100644 --- a/website/source/docs/agent/telemetry.html.md +++ b/website/source/docs/agent/telemetry.html.md @@ -138,6 +138,18 @@ This is a full list of metrics emitted by Consul. Unit Type + + `consul.acl.blocked.service.registration` + This increments whenever a deregistration fails for a service (blocked by an ACL) + requests + counter + + + `consul.acl.blocked.<check|node|service>.registration` + This increments whenever a registration fails for an entity (check, node or service) is blocked by an ACL + requests + counter + `consul.client.rpc` This increments whenever a Consul agent in client mode makes an RPC request to a Consul server. This gives a measure of how much a given agent is loading the Consul servers. Currently, this is only generated by agents in client mode, not Consul servers. From 0241cda6454e7b46cd4046521ddb2f00a0adc422 Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Mon, 9 Jul 2018 14:39:20 +0100 Subject: [PATCH 46/54] Fix formatting issue in config docs. The floating paragraph seems to need additional indentation to work correctly on the markdown parser middle man uses - GitHub got it right before in the preview but the website broke dumping the new config option inline. --- website/source/docs/agent/options.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index 4e2ecb3033..89917768b4 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -822,7 +822,7 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass matching hosts, shuffle the list randomly, and then limit the number of answers to `a_record_limit` (default: no limit). This limit does not apply to SRV records. - In environments where [RFC 3484 Section 6](https://tools.ietf.org/html/rfc3484#section-6) Rule 9 + In environments where [RFC 3484 Section 6](https://tools.ietf.org/html/rfc3484#section-6) Rule 9 is implemented and enforced (i.e. DNS answers are always sorted and therefore never random), clients may need to set this value to `1` to preserve the expected randomized distribution behavior (note: @@ -831,7 +831,7 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass be increasingly uncommon to need to change this value with modern resolvers). - * `enable_additional_node_meta_txt` - + * `enable_additional_node_meta_txt` - When set to true, Consul will add TXT records for Node metadata into the Additional section of the DNS responses for several query types such as SRV queries. When set to false those records are emitted. This does not impact the behavior of those same TXT records when they would be added to the Answer section of the response like when querying with type TXT or ANY. This From d508a6ba7c00ee940c1b965114f7cde4fd6718e2 Mon Sep 17 00:00:00 2001 From: Leonid Stryzhevskyi Date: Mon, 9 Jul 2018 17:28:03 +0300 Subject: [PATCH 47/54] oatpp-consul integration added to Libraries & SDKs page --- website/source/api/libraries-and-sdks.html.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/website/source/api/libraries-and-sdks.html.md b/website/source/api/libraries-and-sdks.html.md index 4e38239f53..3bf8f732ab 100644 --- a/website/source/api/libraries-and-sdks.html.md +++ b/website/source/api/libraries-and-sdks.html.md @@ -79,4 +79,7 @@ the community.
    4. ConsulSwift - Swift client for the Consul HTTP API
      5. +
    5. + oatpp-consul - C++ Consul integration for oatpp applications +
      6. From 6cecf2961d4b9dc5dbdc23e5f9d4e13750259ab9 Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Thu, 5 Jul 2018 22:04:29 -0400 Subject: [PATCH 48/54] Agent/Proxy : Properly passes env variables to child --- agent/proxy/manager.go | 2 +- agent/proxy/manager_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/agent/proxy/manager.go b/agent/proxy/manager.go index e3c6dd7796..e22079b186 100644 --- a/agent/proxy/manager.go +++ b/agent/proxy/manager.go @@ -432,7 +432,7 @@ func (m *Manager) newProxy(mp *local.ManagedProxy) (Proxy, error) { if err := m.configureLogDir(id, &cmd); err != nil { return nil, fmt.Errorf("error configuring proxy logs: %s", err) } - + cmd.Env = os.Environ() // Build the daemon structure proxy.Command = &cmd proxy.ProxyID = id diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 42286493c7..b3269bf10c 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -261,6 +261,46 @@ func TestManagerRun_daemonPid(t *testing.T) { require.NotEmpty(pidRaw) } +// Test to check if the parent and the child processes +// have the same environmental variables + +func TestEnvironproxy(t *testing.T) { + t.Parallel() + + require := require.New(t) + state := local.TestState(t) + m, closer := testManager(t) + defer closer() + m.State = state + defer m.Kill() + + // Add the proxy + td, closer := testTempDir(t) + defer closer() + path := filepath.Join(td, "env-variables") + d := &Daemon{ + Command: helperProcess("parent", path), + Logger: testLogger, + } + var currentEnviron []string + currentEnviron = os.Environ() + // Environmental variable of the helper + if err := d.Start(); err != nil { + t.Error("Daemon failed to start") + defer d.Stop() + envProcess := d.Command.Env + var envData []byte + for _, data := range envProcess { + envData = append(envData, []byte(data)...) + envData = append(envData, []byte("\n")...) + } + t.Logf("Env Data:%s", envProcess) + //Get the parent environmental variables in a file + + require.Equal(currentEnviron, envProcess) + } +} + // Test the Snapshot/Restore works. func TestManagerRun_snapshotRestore(t *testing.T) { t.Parallel() From 94e8ff55cfea24a7a78cd6e67037a10478ffe12f Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Mon, 9 Jul 2018 12:27:34 -0400 Subject: [PATCH 49/54] Proxy/Tests: Added test cases to check env variables --- agent/proxy/manager_test.go | 49 ++++++++++++++++++++++--------------- agent/proxy/proxy_test.go | 45 +++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 21 deletions(-) diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index b3269bf10c..43a0c0f24a 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -5,6 +5,8 @@ import ( "os" "os/exec" "path/filepath" + "reflect" + "sort" "testing" "time" @@ -274,31 +276,38 @@ func TestEnvironproxy(t *testing.T) { m.State = state defer m.Kill() - // Add the proxy + // Add Proxy for the test td, closer := testTempDir(t) defer closer() path := filepath.Join(td, "env-variables") - d := &Daemon{ - Command: helperProcess("parent", path), - Logger: testLogger, - } - var currentEnviron []string - currentEnviron = os.Environ() - // Environmental variable of the helper - if err := d.Start(); err != nil { - t.Error("Daemon failed to start") - defer d.Stop() - envProcess := d.Command.Env - var envData []byte - for _, data := range envProcess { - envData = append(envData, []byte(data)...) - envData = append(envData, []byte("\n")...) - } - t.Logf("Env Data:%s", envProcess) - //Get the parent environmental variables in a file + testStateProxy(t, state, "environTest", helperProcess("environ", path)) - require.Equal(currentEnviron, envProcess) + //Run the manager + go m.Run() + + //Get the environmental variables from the OS + //envCheck := os.Environ() + var fileContent Bytes + var err error + var data Bytes + envData := os.Environ() + sort.Strings(envData) + for _, envVariable := range envData { + data = append(data, envVariable...) + data = append(data, "\n"...) } + // t.Log(data) + retry.Run(t, func(r *retry.R) { + if fileContent, err = ioutil.ReadFile(path); err != nil { + r.Fatalf("No file ya dummy") + } + }) + + t.Logf("Len (data) : %d , Len (fileContent) : %d", len(data), len(fileContent)) + t.Logf("Type (data) : %s Type (fileContent) : %s", reflect.TypeOf(data), reflect.TypeOf(fileContent)) + // t.Logf("File content: \n %s", fileContent) + // t.Logf("Data content: \n %s", data) + require.Equal(fileContent, data) } // Test the Snapshot/Restore works. diff --git a/agent/proxy/proxy_test.go b/agent/proxy/proxy_test.go index 0a92373192..4394433fe7 100644 --- a/agent/proxy/proxy_test.go +++ b/agent/proxy/proxy_test.go @@ -7,7 +7,9 @@ import ( "os" "os/exec" "os/signal" + "sort" "strconv" + "strings" "syscall" "testing" "time" @@ -17,6 +19,22 @@ import ( // *log.Logger instance. var testLogger = log.New(os.Stderr, "logger: ", log.LstdFlags) +//SOrting interface +type Bytes []byte + +//Length method for our Byte interface +func (b Bytes) Len() int { + return len(b) +} + +func (b Bytes) Swap(i, j int) { + b[i], b[j] = b[j], b[i] +} + +func (b Bytes) Less(i, j int) bool { + return b[i] < b[j] +} + // testTempDir returns a temporary directory and a cleanup function. func testTempDir(t *testing.T) (string, func()) { t.Helper() @@ -124,7 +142,6 @@ func TestHelperProcess(t *testing.T) { default: } } - case "stop-kill": // Setup listeners so it is ignored ch := make(chan os.Signal, 1) @@ -139,6 +156,32 @@ func TestHelperProcess(t *testing.T) { } time.Sleep(25 * time.Millisecond) } + // Check if the external process can access the enivironmental variables + case "environ": + stop := make(chan os.Signal, 1) + signal.Notify(stop, os.Interrupt) + defer signal.Stop(stop) + + //Write the environmental variables into the file + path := args[0] + var data Bytes + // sort.Sort(data) + envData := os.Environ() + sort.Strings(envData) + for _, envVariable := range envData { + if strings.Contains(envVariable, "CONNECT_PROXY") { + continue + } + data = append(data, envVariable...) + data = append(data, "\n"...) + } + if err := ioutil.WriteFile(path, data, 0644); err != nil { + t.Fatalf("[Error] File write failed : %s", err) + } + + // Clean up after we receive the signal to exit + defer os.Remove(path) + <-stop case "output": fmt.Fprintf(os.Stdout, "hello stdout\n") From cbf8f14451a8ffbda1ac1e75d1f3925796894901 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Mon, 9 Jul 2018 11:41:58 -0400 Subject: [PATCH 50/54] Ensure TXT RRs always end up in the Additional section except for ANY or TXT queries This also changes where the enforcement of the enable_additional_node_meta_txt configuration gets applied. formatNodeRecord returns the main RRs and the meta/TXT RRs in separate slices. Its then up to the caller to add to the appropriate sections or not. --- agent/dns.go | 60 +++++++++++++-------- agent/dns_test.go | 130 +++++++++++++++++++++++++++++----------------- 2 files changed, 121 insertions(+), 69 deletions(-) diff --git a/agent/dns.go b/agent/dns.go index 2372d16c86..bd5fe820d8 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -376,8 +376,11 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) { } ns = append(ns, nsrr) - glue := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns, false) + glue, meta := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns) extra = append(extra, glue...) + if meta != nil && d.config.NodeMetaTXT { + extra = append(extra, meta...) + } // don't provide more than 3 servers if len(ns) >= 3 { @@ -592,10 +595,15 @@ RPC: n := out.NodeServices.Node edns := req.IsEdns0() != nil addr := d.agent.TranslateAddress(datacenter, n.Address, n.TaggedAddresses) - records := d.formatNodeRecord(out.NodeServices.Node, addr, req.Question[0].Name, qType, d.config.NodeTTL, edns, true) + records, meta := d.formatNodeRecord(out.NodeServices.Node, addr, req.Question[0].Name, qType, d.config.NodeTTL, edns) if records != nil { resp.Answer = append(resp.Answer, records...) } + if meta != nil && (qType == dns.TypeANY || qType == dns.TypeTXT) { + resp.Answer = append(resp.Answer, meta...) + } else if meta != nil && d.config.NodeMetaTXT { + resp.Extra = append(resp.Extra, meta...) + } } // encodeKVasRFC1464 encodes a key-value pair according to RFC1464 @@ -619,8 +627,12 @@ func encodeKVasRFC1464(key, value string) (txt string) { return key + "=" + value } -// formatNodeRecord takes a Node and returns an A, AAAA, TXT or CNAME record -func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns, answer bool) (records []dns.RR) { +// formatNodeRecord takes a Node and returns the RRs associated with that node +// +// The return value is two slices. The first slice is the main answer slice (containing the A, AAAA, CNAME) RRs for the node +// and the second slice contains any TXT RRs created from the node metadata. It is up to the caller to determine where the +// generated RRs should go and if they should be used at all. +func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns bool) (records, meta []dns.RR) { // Parse the IP ip := net.ParseIP(addr) var ipv4 net.IP @@ -681,26 +693,14 @@ func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qTy } } - node_meta_txt := false - - if node == nil { - node_meta_txt = false - } else if answer { - node_meta_txt = true - } else { - // Use configuration when the TXT RR would - // end up in the Additional section of the - // DNS response - node_meta_txt = d.config.NodeMetaTXT - } - - if node_meta_txt { + if node != nil { for key, value := range node.Meta { txt := value if !strings.HasPrefix(strings.ToLower(key), "rfc1035-") { txt = encodeKVasRFC1464(key, value) } - records = append(records, &dns.TXT{ + + meta = append(meta, &dns.TXT{ Hdr: dns.RR_Header{ Name: qName, Rrtype: dns.TypeTXT, @@ -712,7 +712,7 @@ func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qTy } } - return records + return records, meta } // indexRRs populates a map which indexes a given list of RRs by name. NOTE that @@ -1167,9 +1167,21 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode handled[addr] = struct{}{} // Add the node record - records := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns, true) + had_answer := false + records, meta := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns) if records != nil { resp.Answer = append(resp.Answer, records...) + had_answer = true + } + + if meta != nil && (qType == dns.TypeANY || qType == dns.TypeTXT) { + resp.Answer = append(resp.Answer, meta...) + had_answer = true + } else if meta != nil && d.config.NodeMetaTXT { + resp.Extra = append(resp.Extra, meta...) + } + + if had_answer { count++ if count == d.config.ARecordLimit { // We stop only if greater than 0 or we reached the limit @@ -1216,7 +1228,7 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes } // Add the extra record - records := d.formatNodeRecord(node.Node, addr, srvRec.Target, dns.TypeANY, ttl, edns, false) + records, meta := d.formatNodeRecord(node.Node, addr, srvRec.Target, dns.TypeANY, ttl, edns) if len(records) > 0 { // Use the node address if it doesn't differ from the service address if addr == node.Node.Address { @@ -1246,6 +1258,10 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes resp.Extra = append(resp.Extra, records...) } } + + if meta != nil && d.config.NodeMetaTXT { + resp.Extra = append(resp.Extra, meta...) + } } } } diff --git a/agent/dns_test.go b/agent/dns_test.go index bc6fdaadb0..48143d0203 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -178,6 +178,9 @@ func TestDNS_NodeLookup(t *testing.T) { TaggedAddresses: map[string]string{ "wan": "127.0.0.2", }, + NodeMeta: map[string]string{ + "key": "value", + }, } var out struct{} @@ -190,24 +193,40 @@ func TestDNS_NodeLookup(t *testing.T) { c := new(dns.Client) in, _, err := c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } + require.NoError(t, err) + require.Len(t, in.Answer, 2) + require.Len(t, in.Extra, 0) aRec, ok := in.Answer[0].(*dns.A) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if aRec.A.String() != "127.0.0.1" { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if aRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Answer[0]) - } + require.True(t, ok, "First answer is not an A record") + require.Equal(t, "127.0.0.1", aRec.A.String()) + require.Equal(t, uint32(0), aRec.Hdr.Ttl) + + txt, ok := in.Answer[1].(*dns.TXT) + require.True(t, ok, "Second answer is not a TXT record") + require.Len(t, txt.Txt, 1) + require.Equal(t, "key=value", txt.Txt[0]) + + // Re-do the query, but only for an A RR + + m = new(dns.Msg) + m.SetQuestion("foo.node.consul.", dns.TypeA) + + c = new(dns.Client) + in, _, err = c.Exchange(m, a.DNSAddr()) + require.NoError(t, err) + require.Len(t, in.Answer, 1) + require.Len(t, in.Extra, 1) + + aRec, ok = in.Answer[0].(*dns.A) + require.True(t, ok, "Answer is not an A record") + require.Equal(t, "127.0.0.1", aRec.A.String()) + require.Equal(t, uint32(0), aRec.Hdr.Ttl) + + txt, ok = in.Extra[0].(*dns.TXT) + require.True(t, ok, "Extra record is not a TXT record") + require.Len(t, txt.Txt, 1) + require.Equal(t, "key=value", txt.Txt[0]) // Re-do the query, but specify the DC m = new(dns.Msg) @@ -215,24 +234,17 @@ func TestDNS_NodeLookup(t *testing.T) { c = new(dns.Client) in, _, err = c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } + require.NoError(t, err) + require.Len(t, in.Answer, 2) + require.Len(t, in.Extra, 0) aRec, ok = in.Answer[0].(*dns.A) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if aRec.A.String() != "127.0.0.1" { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if aRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Answer[0]) - } + require.True(t, ok, "First answer is not an A record") + require.Equal(t, "127.0.0.1", aRec.A.String()) + require.Equal(t, uint32(0), aRec.Hdr.Ttl) + + txt, ok = in.Answer[1].(*dns.TXT) + require.True(t, ok, "Second answer is not a TXT record") // lookup a non-existing node, we should receive a SOA m = new(dns.Msg) @@ -240,22 +252,11 @@ func TestDNS_NodeLookup(t *testing.T) { c = new(dns.Client) in, _, err = c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Ns) != 1 { - t.Fatalf("Bad: %#v %#v", in, len(in.Answer)) - } - + require.NoError(t, err) + require.Len(t, in.Ns, 1) soaRec, ok := in.Ns[0].(*dns.SOA) - if !ok { - t.Fatalf("Bad: %#v", in.Ns[0]) - } - if soaRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Ns[0]) - } - + require.True(t, ok, "NS RR is not a SOA record") + require.Equal(t, uint32(0), soaRec.Hdr.Ttl) } func TestDNS_CaseInsensitiveNodeLookup(t *testing.T) { @@ -598,6 +599,41 @@ func TestDNS_NodeLookup_ANY_DontSuppressTXT(t *testing.T) { verify.Values(t, "answer", in.Answer, wantAnswer) } +func TestDNS_NodeLookup_A_SuppressTXT(t *testing.T) { + a := NewTestAgent(t.Name(), `dns_config = { enable_additional_node_meta_txt = false }`) + defer a.Shutdown() + + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "127.0.0.1", + NodeMeta: map[string]string{ + "key": "value", + }, + } + + var out struct{} + require.NoError(t, a.RPC("Catalog.Register", args, &out)) + + m := new(dns.Msg) + m.SetQuestion("bar.node.consul.", dns.TypeA) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + require.NoError(t, err) + + wantAnswer := []dns.RR{ + &dns.A{ + Hdr: dns.RR_Header{Name: "bar.node.consul.", Rrtype: dns.TypeA, Class: dns.ClassINET, Rdlength: 0x4}, + A: []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1 + }, + } + verify.Values(t, "answer", in.Answer, wantAnswer) + + // ensure TXT RR suppression + require.Len(t, in.Extra, 0) +} + func TestDNS_EDNS0(t *testing.T) { t.Parallel() a := NewTestAgent(t.Name(), "") From 300330e24b72f51105fcf0854e2e996d0a69baad Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Mon, 9 Jul 2018 12:46:10 -0400 Subject: [PATCH 51/54] Agent/Proxy: Formatting and test cases fix --- agent/consul/server.go | 2 +- agent/proxy/manager.go | 3 +++ agent/proxy/manager_test.go | 14 +++++--------- agent/proxy/proxy_test.go | 28 ++++++++-------------------- 4 files changed, 17 insertions(+), 30 deletions(-) diff --git a/agent/consul/server.go b/agent/consul/server.go index 4520b11114..e153f9b28d 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -427,7 +427,7 @@ func NewServerLogger(config *Config, logger *log.Logger, tokens *token.Store) (* } go s.Flood(nil, portFn, s.serfWAN) } - + // Start enterprise specific functionality if err := s.startEnterprise(); err != nil { s.Shutdown() diff --git a/agent/proxy/manager.go b/agent/proxy/manager.go index e22079b186..724ed39b4d 100644 --- a/agent/proxy/manager.go +++ b/agent/proxy/manager.go @@ -432,7 +432,10 @@ func (m *Manager) newProxy(mp *local.ManagedProxy) (Proxy, error) { if err := m.configureLogDir(id, &cmd); err != nil { return nil, fmt.Errorf("error configuring proxy logs: %s", err) } + + // Pass in the environmental variables for this proxy process cmd.Env = os.Environ() + // Build the daemon structure proxy.Command = &cmd proxy.ProxyID = id diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 43a0c0f24a..10234e90d2 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -5,7 +5,6 @@ import ( "os" "os/exec" "path/filepath" - "reflect" "sort" "testing" "time" @@ -286,27 +285,24 @@ func TestEnvironproxy(t *testing.T) { go m.Run() //Get the environmental variables from the OS - //envCheck := os.Environ() - var fileContent Bytes + var fileContent []byte var err error - var data Bytes + var data []byte envData := os.Environ() sort.Strings(envData) for _, envVariable := range envData { data = append(data, envVariable...) data = append(data, "\n"...) } - // t.Log(data) + + // Check if the file written to from the spawned process + // has the necessary environmental variable data retry.Run(t, func(r *retry.R) { if fileContent, err = ioutil.ReadFile(path); err != nil { r.Fatalf("No file ya dummy") } }) - t.Logf("Len (data) : %d , Len (fileContent) : %d", len(data), len(fileContent)) - t.Logf("Type (data) : %s Type (fileContent) : %s", reflect.TypeOf(data), reflect.TypeOf(fileContent)) - // t.Logf("File content: \n %s", fileContent) - // t.Logf("Data content: \n %s", data) require.Equal(fileContent, data) } diff --git a/agent/proxy/proxy_test.go b/agent/proxy/proxy_test.go index 4394433fe7..0ac0446a8f 100644 --- a/agent/proxy/proxy_test.go +++ b/agent/proxy/proxy_test.go @@ -19,22 +19,6 @@ import ( // *log.Logger instance. var testLogger = log.New(os.Stderr, "logger: ", log.LstdFlags) -//SOrting interface -type Bytes []byte - -//Length method for our Byte interface -func (b Bytes) Len() int { - return len(b) -} - -func (b Bytes) Swap(i, j int) { - b[i], b[j] = b[j], b[i] -} - -func (b Bytes) Less(i, j int) bool { - return b[i] < b[j] -} - // testTempDir returns a temporary directory and a cleanup function. func testTempDir(t *testing.T) (string, func()) { t.Helper() @@ -162,14 +146,17 @@ func TestHelperProcess(t *testing.T) { signal.Notify(stop, os.Interrupt) defer signal.Stop(stop) - //Write the environmental variables into the file + //Get the path for the file to be written to path := args[0] - var data Bytes - // sort.Sort(data) + var data []byte + + //Get the environmental variables envData := os.Environ() + + //Sort the env data for easier comparison sort.Strings(envData) for _, envVariable := range envData { - if strings.Contains(envVariable, "CONNECT_PROXY") { + if strings.HasPrefix(envVariable, "CONSUL") || strings.HasPrefix(envVariable, "CONNECT") { continue } data = append(data, envVariable...) @@ -181,6 +168,7 @@ func TestHelperProcess(t *testing.T) { // Clean up after we receive the signal to exit defer os.Remove(path) + <-stop case "output": From 371f0c3d5f792287e9b36972f7ea6beda7317fc3 Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Mon, 9 Jul 2018 13:18:57 -0400 Subject: [PATCH 52/54] Tests/Proxy : Changed function name to match the system being tested. --- agent/proxy/manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 10234e90d2..d9e63b6c63 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -265,7 +265,7 @@ func TestManagerRun_daemonPid(t *testing.T) { // Test to check if the parent and the child processes // have the same environmental variables -func TestEnvironproxy(t *testing.T) { +func TestManagerPassesEnvironment(t *testing.T) { t.Parallel() require := require.New(t) From c146f837bdcbe041f10d83f826965450a36a6066 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Tue, 10 Jul 2018 08:52:15 -0400 Subject: [PATCH 53/54] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27926ca82d..0969a6055e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ BUG FIXES: * agent: Intention read endpoint returns a 400 on invalid UUID [[GH-4297](https://github.com/hashicorp/consul/issues/4297)] * agent: Service registration with "services" does not error on Connect upstream configuration. [[GH-4308](https://github.com/hashicorp/consul/issues/4308)] * catalog: Ensure all registered services have IDs and auto-gen them if need be. [[GH-4249](https://github.com/hashicorp/consul/issues/4249)] +* dns: Ensure that TXT RRs dont get put in the Answer section for A/AAAA queries. [[GH-4354](https://github.com/hashicorp/consul/issues/4354)] ## 1.2.0 (June 26, 2018) From ba24bdd32b54791e1cb8dcf925894934dcbeaf80 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Tue, 10 Jul 2018 09:02:06 -0400 Subject: [PATCH 54/54] Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0969a6055e..b01c4019cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## UNRELEASED +IMPROVEMENTS: + +* acl: Prevented multiple ACL token refresh operations from occurring simultaneously. [[GH-3524](https://github.com/hashicorp/consul/issues/3524)] +* acl: Add async-cache down policy mode to always do ACL token refreshes in the background to reduce latency. [[GH-3524](https://github.com/hashicorp/consul/issues/3524)] + BUG FIXES: * api: Intention APIs parse error response body for error message. [[GH-4297](https://github.com/hashicorp/consul/issues/4297)]