diff --git a/client/allocrunner/taskrunner/sids_hook.go b/client/allocrunner/taskrunner/sids_hook.go index b9768f05c..e2aaa7fca 100644 --- a/client/allocrunner/taskrunner/sids_hook.go +++ b/client/allocrunner/taskrunner/sids_hook.go @@ -190,30 +190,25 @@ func (h *sidsHook) kill(ctx context.Context, err error) { // tryDerive loops forever until a token is created, or ctx is done. func (h *sidsHook) tryDerive(ctx context.Context, ch chan<- string) { - // Derive the SI token using the name of the proxied / native task, not the - // name of the literal sidecar task. The virtual ACL policy of the SI token - // is oriented this way. - siTaskName := h.task.Kind.Value() - for attempt := 0; backoff(ctx, attempt); attempt++ { - tokens, err := h.sidsClient.DeriveSITokens(h.alloc, []string{siTaskName}) + tokens, err := h.sidsClient.DeriveSITokens(h.alloc, []string{h.task.Name}) switch { case err == nil: // nothing broke and we can return the token for the task - ch <- tokens[siTaskName] + ch <- tokens[h.task.Name] return case structs.IsServerSide(err): // the error is known to be a server problem, just die - h.logger.Error("failed to derive SI token", "error", err, "task", h.task.Name, "si_task", siTaskName, "server_side", true) + h.logger.Error("failed to derive SI token", "error", err, "task", h.task.Name, "server_side", true) h.kill(ctx, errors.Wrap(err, "consul: failed to derive SI token")) case !structs.IsRecoverable(err): // the error is known not to be recoverable, just die - h.logger.Error("failed to derive SI token", "error", err, "task", h.task.Name, "si_task", siTaskName, "recoverable", false) + h.logger.Error("failed to derive SI token", "error", err, "task", h.task.Name, "recoverable", false) h.kill(ctx, errors.Wrap(err, "consul: failed to derive SI token")) default: diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index 20dc536a8..44530a4f5 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -1192,12 +1192,11 @@ func TestTaskRunner_DeriveSIToken_Retry(t *testing.T) { // control when we get a Consul SI token token := "12345678-1234-1234-1234-1234567890" - siTaskName := task.Kind.Value() deriveCount := 0 deriveFn := func(*structs.Allocation, []string) (map[string]string, error) { if deriveCount > 0 { - return map[string]string{siTaskName: token}, nil + return map[string]string{task.Name: token}, nil } deriveCount++ return nil, structs.NewRecoverableError(errors.New("try again later"), true) @@ -1252,9 +1251,8 @@ func TestTaskRunner_DeriveSIToken_Unrecoverable(t *testing.T) { defer cleanup() // SI token derivation suffers a non-retryable error - siTaskName := task.Kind.Value() siClient := trConfig.ConsulSI.(*consulapi.MockServiceIdentitiesClient) - siClient.SetDeriveTokenError(alloc.ID, []string{siTaskName}, errors.New("non-recoverable")) + siClient.SetDeriveTokenError(alloc.ID, []string{task.Name}, errors.New("non-recoverable")) tr, err := NewTaskRunner(trConfig) r.NoError(err) diff --git a/command/agent/consul/acl_testing.go b/command/agent/consul/acl_testing.go index 05045ae7a..294810122 100644 --- a/command/agent/consul/acl_testing.go +++ b/command/agent/consul/acl_testing.go @@ -35,6 +35,7 @@ func NewMockACLsAPI(l hclog.Logger) *MockACLsAPI { } } +// Example Consul policies for use in tests. const ( ExamplePolicyID1 = "a7c86856-0af5-4ab5-8834-03f4517e5564" ExamplePolicyID2 = "ffa1b66c-967d-4468-8775-c687b5cfc16e" @@ -47,6 +48,7 @@ func (m *MockACLsAPI) PolicyRead(policyID string, _ *api.QueryOptions) (*api.ACL case ExamplePolicyID1: return &api.ACLPolicy{ ID: ExamplePolicyID1, + Name: "example-policy-1", Rules: `service "service1" { policy = "write" }`, }, nil, nil @@ -69,44 +71,118 @@ service "service2" { policy = "write" }`, } } +// Example Consul roles for use in tests. const ( - ExampleOperatorToken1 = "59c219c2-47e4-43f3-bb45-258fd13f59d5" - ExampleOperatorToken2 = "868cc216-e123-4c2b-b362-f4d4c087de8e" - ExampleOperatorToken3 = "6177d1b9-c0f6-4118-b891-d818a3cb80b1" + ExampleRoleID1 = "e569a3a8-7dfb-b024-e492-e790fe3c4183" + ExampleRoleID2 = "88c825f4-d0da-1c2b-0c1c-cc9fe84c4468" + ExampleRoleID3 = "b19b2058-6205-6dff-d2b0-470f29b8e627" +) + +func (m *MockACLsAPI) RoleRead(roleID string, _ *api.QueryOptions) (*api.ACLRole, *api.QueryMeta, error) { + switch roleID { + case ExampleRoleID1: + return &api.ACLRole{ + ID: ExampleRoleID1, + Name: "example-role-1", + Policies: []*api.ACLRolePolicyLink{{ + ID: ExamplePolicyID1, + Name: "example-policy-1", + }}, + ServiceIdentities: nil, // would it ever make sense ? + }, nil, nil + case ExampleRoleID2: + return &api.ACLRole{ + ID: ExampleRoleID2, + Name: "example-role-2", + Policies: []*api.ACLRolePolicyLink{{ + ID: ExamplePolicyID2, + Name: "example-policy-2", + }}, + ServiceIdentities: nil, + }, nil, nil + case ExampleRoleID3: + return &api.ACLRole{ + ID: ExampleRoleID3, + Name: "example-role-3", + Policies: nil, // todo + ServiceIdentities: nil, // todo + ModifyIndex: 0, + }, nil, nil + default: + return nil, nil, nil + } +} + +// Example Consul "operator" tokens for use in tests. + +const ( + ExampleOperatorTokenID0 = "de591604-86eb-1e6f-8b44-d4db752921ae" + ExampleOperatorTokenID1 = "59c219c2-47e4-43f3-bb45-258fd13f59d5" + ExampleOperatorTokenID2 = "868cc216-e123-4c2b-b362-f4d4c087de8e" + ExampleOperatorTokenID3 = "6177d1b9-c0f6-4118-b891-d818a3cb80b1" + ExampleOperatorTokenID4 = "754ae26c-f3cc-e088-d486-9c0d20f5eaea" +) + +var ( + ExampleOperatorToken0 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID0, + AccessorID: "228865c6-3bf6-6683-df03-06dea2779088 ", + Description: "Operator Token 0", + } + + ExampleOperatorToken1 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID1, + AccessorID: "e341bacd-535e-417c-8f45-f88d7faffcaf", + Description: "Operator Token 1", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID1, + }}, + } + + ExampleOperatorToken2 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID2, + AccessorID: "615b4d77-5164-4ec6-b616-24c0b24ac9cb", + Description: "Operator Token 2", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID2, + }}, + } + + ExampleOperatorToken3 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID3, + AccessorID: "6b7de0d7-15f7-45b4-95eb-fb775bfe3fdc", + Description: "Operator Token 3", + Policies: []*api.ACLTokenPolicyLink{{ + ID: ExamplePolicyID3, + }}, + } + + ExampleOperatorToken4 = &api.ACLToken{ + SecretID: ExampleOperatorTokenID4, + AccessorID: "7b5fdb1a-71e5-f3d8-2cfe-448d973f327d", + Description: "Operator Token 4", + Policies: nil, // no direct policy, only roles + Roles: []*api.ACLTokenRoleLink{{ + ID: ExampleRoleID1, + Name: "example-role-1", + }}, + } ) func (m *MockACLsAPI) TokenReadSelf(q *api.QueryOptions) (*api.ACLToken, *api.QueryMeta, error) { switch q.Token { - case ExampleOperatorToken1: - return &api.ACLToken{ - SecretID: ExampleOperatorToken1, - AccessorID: "e341bacd-535e-417c-8f45-f88d7faffcaf", - Description: "operator token 1", - Policies: []*api.ACLTokenPolicyLink{{ - ID: ExamplePolicyID1, - }}, - }, nil, nil + case ExampleOperatorTokenID1: + return ExampleOperatorToken1, nil, nil - case ExampleOperatorToken2: - return &api.ACLToken{ - SecretID: ExampleOperatorToken2, - AccessorID: "615b4d77-5164-4ec6-b616-24c0b24ac9cb", - Description: "operator token 2", - Policies: []*api.ACLTokenPolicyLink{{ - ID: ExamplePolicyID2, - }}, - }, nil, nil + case ExampleOperatorTokenID2: + return ExampleOperatorToken2, nil, nil - case ExampleOperatorToken3: - return &api.ACLToken{ - SecretID: ExampleOperatorToken3, - AccessorID: "6b7de0d7-15f7-45b4-95eb-fb775bfe3fdc", - Description: "operator token 3", - Policies: []*api.ACLTokenPolicyLink{{ - ID: ExamplePolicyID3, - }}, - }, nil, nil + case ExampleOperatorTokenID3: + return ExampleOperatorToken3, nil, nil + + case ExampleOperatorTokenID4: + return ExampleOperatorToken4, nil, nil default: return nil, nil, errors.New("no such token") diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index 72ca3d5d5..333f510b5 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -93,15 +93,14 @@ type AgentAPI interface { UpdateTTL(id, output, status string) error } -// ACLsAPI is the consul/api.ACL API used by Nomad Server. +// ACLsAPI is the consul/api.ACL API subset used by Nomad Server. type ACLsAPI interface { - // todo: RoleRead (...) - // We are looking up by [operator token] SecretID, which implies we need // to use this method instead of the normal TokenRead, which can only be // used to lookup tokens by their AccessorID. TokenReadSelf(q *api.QueryOptions) (*api.ACLToken, *api.QueryMeta, error) PolicyRead(policyID string, q *api.QueryOptions) (*api.ACLPolicy, *api.QueryMeta, error) + RoleRead(roleID string, q *api.QueryOptions) (*api.ACLRole, *api.QueryMeta, error) TokenCreate(partial *api.ACLToken, q *api.WriteOptions) (*api.ACLToken, *api.WriteMeta, error) TokenDelete(accessorID string, q *api.WriteOptions) (*api.WriteMeta, error) TokenList(q *api.QueryOptions) ([]*api.ACLTokenListEntry, *api.QueryMeta, error) diff --git a/nomad/consul.go b/nomad/consul.go index 9df1409e6..4495adfb6 100644 --- a/nomad/consul.go +++ b/nomad/consul.go @@ -9,7 +9,6 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/consul/api" "github.com/hashicorp/go-hclog" - "github.com/hashicorp/hcl" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/nomad/structs" "github.com/pkg/errors" @@ -49,36 +48,6 @@ const ( ConsulPolicyWrite = "write" ) -// ConsulServiceRule represents a policy for a service -type ConsulServiceRule struct { - Name string `hcl:",key"` - Policy string -} - -type ConsulPolicy struct { - Services []*ConsulServiceRule `hcl:"service,expand"` - ServicePrefixes []*ConsulServiceRule `hcl:"service_prefix,expand"` -} - -func (cp *ConsulPolicy) IsEmpty() bool { - if cp == nil { - return true - } - return len(cp.Services) == 0 && len(cp.ServicePrefixes) == 0 -} - -func ParseConsulPolicy(s string) (*ConsulPolicy, error) { - cp := new(ConsulPolicy) - if err := hcl.Decode(cp, s); err != nil { - return nil, errors.Wrap(err, "failed to parse ACL policy") - } - if cp.IsEmpty() { - // the only use case for now, may as well validate asap - return nil, errors.New("consul policy contains no service rules") - } - return cp, nil -} - type ServiceIdentityIndex struct { ClusterID string AllocID string @@ -176,72 +145,6 @@ func (c *consulACLsAPI) CheckSIPolicy(_ context.Context, task, secretID string) return nil } -func (c *consulACLsAPI) hasSufficientPolicy(task string, token *api.ACLToken) (bool, error) { - - for _, policyRef := range token.Policies { - if allowable, err := c.policyAllowsServiceWrite(task, policyRef.ID); err != nil { - return false, err - } else if allowable { - return true, nil - } - } - - // todo: probably also need to go through each role and check all those - - return false, nil -} - -// policyAllowsServiceWrite -func (c *consulACLsAPI) policyAllowsServiceWrite(task string, policyID string) (bool, error) { - policy, _, err := c.aclClient.PolicyRead(policyID, &api.QueryOptions{ - AllowStale: false, - }) - if err != nil { - return false, err - } - - // compare policy to the necessary permission for service write - // e.g. service "db" { policy = "write" } - // e.g. service_prefix "" { policy == "write" } - cp, err := ParseConsulPolicy(policy.Rules) - if err != nil { - return false, err - } - - if c.allowsServiceWrite(task, cp) { - return true, nil - } - - return false, nil -} - -const ( - serviceNameWildcard = "*" -) - -func (_ *consulACLsAPI) allowsServiceWrite(task string, cp *ConsulPolicy) bool { - for _, service := range cp.Services { - name := strings.ToLower(service.Name) - policy := strings.ToLower(service.Policy) - if policy == ConsulPolicyWrite { - if name == task || name == serviceNameWildcard { - return true - } - } - } - - for _, servicePrefix := range cp.ServicePrefixes { - prefix := strings.ToLower(servicePrefix.Name) - policy := strings.ToLower(servicePrefix.Policy) - if policy == ConsulPolicyWrite { - if strings.HasPrefix(task, prefix) { - return true - } - } - } - return false -} - func (c *consulACLsAPI) CreateToken(ctx context.Context, sii ServiceIdentityIndex) (*structs.SIToken, error) { defer metrics.MeasureSince([]string{"nomad", "consul", "create_token"}, time.Now()) @@ -254,9 +157,12 @@ func (c *consulACLsAPI) CreateToken(ctx context.Context, sii ServiceIdentityInde // todo: rate limiting + // the token created must be for the service, not the sidecar of the service + // https://www.consul.io/docs/acl/acl-system.html#acl-service-identities + serviceName := strings.TrimPrefix(sii.TaskName, structs.ConnectProxyPrefix+"-") partial := &api.ACLToken{ Description: sii.Description(), - ServiceIdentities: []*api.ACLServiceIdentity{{ServiceName: sii.TaskName}}, + ServiceIdentities: []*api.ACLServiceIdentity{{ServiceName: serviceName}}, } token, _, err := c.aclClient.TokenCreate(partial, nil) diff --git a/nomad/consul_policy.go b/nomad/consul_policy.go new file mode 100644 index 000000000..4f8a62379 --- /dev/null +++ b/nomad/consul_policy.go @@ -0,0 +1,123 @@ +package nomad + +import ( + "strings" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/hcl" + "github.com/pkg/errors" +) + +// ConsulServiceRule represents a policy for a service +type ConsulServiceRule struct { + Name string `hcl:",key"` + Policy string +} + +type ConsulPolicy struct { + Services []*ConsulServiceRule `hcl:"service,expand"` + ServicePrefixes []*ConsulServiceRule `hcl:"service_prefix,expand"` +} + +func (cp *ConsulPolicy) IsEmpty() bool { + if cp == nil { + return true + } + return len(cp.Services) == 0 && len(cp.ServicePrefixes) == 0 +} + +func ParseConsulPolicy(s string) (*ConsulPolicy, error) { + cp := new(ConsulPolicy) + if err := hcl.Decode(cp, s); err != nil { + return nil, errors.Wrap(err, "failed to parse ACL policy") + } + if cp.IsEmpty() { + // the only use case for now, may as well validate asap + return nil, errors.New("consul policy contains no service rules") + } + return cp, nil +} + +func (c *consulACLsAPI) hasSufficientPolicy(task string, token *api.ACLToken) (bool, error) { + // check each policy directly attached to the token + for _, policyRef := range token.Policies { + if allowable, err := c.policyAllowsServiceWrite(task, policyRef.ID); err != nil { + return false, err + } else if allowable { + return true, nil + } + } + + // check each policy on each role attached to the token + for _, roleLink := range token.Roles { + role, _, err := c.aclClient.RoleRead(roleLink.ID, &api.QueryOptions{ + AllowStale: false, + }) + if err != nil { + return false, err + } + + for _, policyLink := range role.Policies { + allowable, err := c.policyAllowsServiceWrite(task, policyLink.ID) + if err != nil { + return false, err + } + if allowable { + return true, nil + } + } + } + + return false, nil +} + +// policyAllowsServiceWrite +func (c *consulACLsAPI) policyAllowsServiceWrite(task string, policyID string) (bool, error) { + policy, _, err := c.aclClient.PolicyRead(policyID, &api.QueryOptions{ + AllowStale: false, + }) + if err != nil { + return false, err + } + + // compare policy to the necessary permission for service write + // e.g. service "db" { policy = "write" } + // e.g. service_prefix "" { policy == "write" } + cp, err := ParseConsulPolicy(policy.Rules) + if err != nil { + return false, err + } + + if c.allowsServiceWrite(task, cp) { + return true, nil + } + + return false, nil +} + +const ( + serviceNameWildcard = "*" +) + +func (_ *consulACLsAPI) allowsServiceWrite(task string, cp *ConsulPolicy) bool { + for _, service := range cp.Services { + name := strings.ToLower(service.Name) + policy := strings.ToLower(service.Policy) + if policy == ConsulPolicyWrite { + if name == task || name == serviceNameWildcard { + return true + } + } + } + + for _, servicePrefix := range cp.ServicePrefixes { + prefix := strings.ToLower(servicePrefix.Name) + policy := strings.ToLower(servicePrefix.Policy) + if policy == ConsulPolicyWrite { + if strings.HasPrefix(task, prefix) { + return true + } + } + } + return false +} diff --git a/nomad/consul_policy_test.go b/nomad/consul_policy_test.go new file mode 100644 index 000000000..35f5d407b --- /dev/null +++ b/nomad/consul_policy_test.go @@ -0,0 +1,203 @@ +package nomad + +import ( + "testing" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/nomad/command/agent/consul" + "github.com/hashicorp/nomad/helper/testlog" + "github.com/stretchr/testify/require" +) + +func TestConsulPolicy_ParseConsulPolicy(t *testing.T) { + t.Parallel() + + try := func(t *testing.T, text string, expPolicy *ConsulPolicy, expErr string) { + policy, err := ParseConsulPolicy(text) + if expErr != "" { + require.EqualError(t, err, expErr) + require.True(t, policy.IsEmpty()) + } else { + require.NoError(t, err) + require.Equal(t, expPolicy, policy) + } + } + + t.Run("service", func(t *testing.T) { + text := `service "web" { policy = "read" }` + exp := &ConsulPolicy{ + Services: []*ConsulServiceRule{{Name: "web", Policy: "read"}}, + ServicePrefixes: []*ConsulServiceRule(nil), + } + try(t, text, exp, "") + }) + + t.Run("service_prefix", func(t *testing.T) { + text := `service_prefix "data" { policy = "write" }` + exp := &ConsulPolicy{ + Services: []*ConsulServiceRule(nil), + ServicePrefixes: []*ConsulServiceRule{{Name: "data", Policy: "write"}}, + } + try(t, text, exp, "") + }) + + t.Run("empty", func(t *testing.T) { + text := `` + expErr := "consul policy contains no service rules" + try(t, text, nil, expErr) + }) + + t.Run("malformed", func(t *testing.T) { + text := `this is not valid HCL!` + expErr := "failed to parse ACL policy: At 1:22: illegal char" + try(t, text, nil, expErr) + }) +} + +func TestConsulPolicy_IsEmpty(t *testing.T) { + t.Parallel() + + try := func(t *testing.T, cp *ConsulPolicy, exp bool) { + result := cp.IsEmpty() + require.Equal(t, exp, result) + } + + t.Run("nil", func(t *testing.T) { + cp := (*ConsulPolicy)(nil) + try(t, cp, true) + }) + + t.Run("empty slices", func(t *testing.T) { + cp := &ConsulPolicy{ + Services: []*ConsulServiceRule(nil), + ServicePrefixes: []*ConsulServiceRule(nil), + } + try(t, cp, true) + }) + + t.Run("services nonempty", func(t *testing.T) { + cp := &ConsulPolicy{ + Services: []*ConsulServiceRule{{Name: "example", Policy: "write"}}, + } + try(t, cp, false) + }) + + t.Run("service_prefixes nonempty", func(t *testing.T) { + cp := &ConsulPolicy{ + ServicePrefixes: []*ConsulServiceRule{{Name: "pre", Policy: "read"}}, + } + try(t, cp, false) + }) +} + +func TestConsulACLsAPI_allowsServiceWrite(t *testing.T) { + t.Parallel() + + try := func(t *testing.T, task string, cp *ConsulPolicy, exp bool) { + cAPI := new(consulACLsAPI) + result := cAPI.allowsServiceWrite(task, cp) + require.Equal(t, exp, result) + } + + makeCP := func(services [][2]string, prefixes [][2]string) *ConsulPolicy { + serviceRules := make([]*ConsulServiceRule, 0, len(services)) + for _, service := range services { + serviceRules = append(serviceRules, &ConsulServiceRule{Name: service[0], Policy: service[1]}) + } + prefixRules := make([]*ConsulServiceRule, 0, len(prefixes)) + for _, prefix := range prefixes { + prefixRules = append(prefixRules, &ConsulServiceRule{Name: prefix[0], Policy: prefix[1]}) + } + return &ConsulPolicy{Services: serviceRules, ServicePrefixes: prefixRules} + } + + t.Run("matching service policy write", func(t *testing.T) { + try(t, "task1", makeCP( + [][2]string{{"task1", "write"}}, + nil, + ), true) + }) + + t.Run("matching service policy read", func(t *testing.T) { + try(t, "task1", makeCP( + [][2]string{{"task1", "read"}}, + nil, + ), false) + }) + + t.Run("wildcard service policy write", func(t *testing.T) { + try(t, "task1", makeCP( + [][2]string{{"*", "write"}}, + nil, + ), true) + }) + + t.Run("wrong service policy write", func(t *testing.T) { + try(t, "other1", makeCP( + [][2]string{{"task1", "write"}}, + nil, + ), false) + }) + + t.Run("matching prefix policy write", func(t *testing.T) { + try(t, "task-one", makeCP( + nil, + [][2]string{{"task-", "write"}}, + ), true) + }) + + t.Run("matching prefix policy read", func(t *testing.T) { + try(t, "task-one", makeCP( + nil, + [][2]string{{"task-", "read"}}, + ), false) + }) + + t.Run("empty prefix policy write", func(t *testing.T) { + try(t, "task-one", makeCP( + nil, + [][2]string{{"", "write"}}, + ), true) + }) + + t.Run("late matching service", func(t *testing.T) { + try(t, "task1", makeCP( + [][2]string{{"task0", "write"}, {"task1", "write"}}, + nil, + ), true) + }) + + t.Run("late matching prefix", func(t *testing.T) { + try(t, "task-one", makeCP( + nil, + [][2]string{{"foo-", "write"}, {"task-", "write"}}, + ), true) + }) +} + +func TestConsulACLsAPI_hasSufficientPolicy(t *testing.T) { + t.Parallel() + + try := func(t *testing.T, task string, token *api.ACLToken, exp bool) { + logger := testlog.HCLogger(t) + cAPI := &consulACLsAPI{ + aclClient: consul.NewMockACLsAPI(logger), + logger: logger, + } + result, err := cAPI.hasSufficientPolicy(task, token) + require.NoError(t, err) + require.Equal(t, exp, result) + } + + t.Run("no useful policy or role", func(t *testing.T) { + try(t, "service1", consul.ExampleOperatorToken0, false) + }) + + t.Run("working policy only", func(t *testing.T) { + try(t, "service1", consul.ExampleOperatorToken1, true) + }) + + t.Run("working role only", func(t *testing.T) { + try(t, "service1", consul.ExampleOperatorToken4, true) + }) +} diff --git a/nomad/consul_test.go b/nomad/consul_test.go index 8a5d844e4..2f9d0f48f 100644 --- a/nomad/consul_test.go +++ b/nomad/consul_test.go @@ -13,87 +13,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestConsulPolicy_ParseConsulPolicy(t *testing.T) { - t.Parallel() - - try := func(t *testing.T, text string, expPolicy *ConsulPolicy, expErr string) { - policy, err := ParseConsulPolicy(text) - if expErr != "" { - require.EqualError(t, err, expErr) - require.True(t, policy.IsEmpty()) - } else { - require.NoError(t, err) - require.Equal(t, expPolicy, policy) - } - } - - t.Run("service", func(t *testing.T) { - text := `service "web" { policy = "read" }` - exp := &ConsulPolicy{ - Services: []*ConsulServiceRule{{Name: "web", Policy: "read"}}, - ServicePrefixes: []*ConsulServiceRule(nil), - } - try(t, text, exp, "") - }) - - t.Run("service_prefix", func(t *testing.T) { - text := `service_prefix "data" { policy = "write" }` - exp := &ConsulPolicy{ - Services: []*ConsulServiceRule(nil), - ServicePrefixes: []*ConsulServiceRule{{Name: "data", Policy: "write"}}, - } - try(t, text, exp, "") - }) - - t.Run("empty", func(t *testing.T) { - text := `` - expErr := "consul policy contains no service rules" - try(t, text, nil, expErr) - }) - - t.Run("malformed", func(t *testing.T) { - text := `this is not valid HCL!` - expErr := "failed to parse ACL policy: At 1:22: illegal char" - try(t, text, nil, expErr) - }) -} - -func TestConsulPolicy_IsEmpty(t *testing.T) { - t.Parallel() - - try := func(t *testing.T, cp *ConsulPolicy, exp bool) { - result := cp.IsEmpty() - require.Equal(t, exp, result) - } - - t.Run("nil", func(t *testing.T) { - cp := (*ConsulPolicy)(nil) - try(t, cp, true) - }) - - t.Run("empty slices", func(t *testing.T) { - cp := &ConsulPolicy{ - Services: []*ConsulServiceRule(nil), - ServicePrefixes: []*ConsulServiceRule(nil), - } - try(t, cp, true) - }) - - t.Run("services nonempty", func(t *testing.T) { - cp := &ConsulPolicy{ - Services: []*ConsulServiceRule{{Name: "example", Policy: "write"}}, - } - try(t, cp, false) - }) - - t.Run("service_prefixes nonempty", func(t *testing.T) { - cp := &ConsulPolicy{ - ServicePrefixes: []*ConsulServiceRule{{Name: "pre", Policy: "read"}}, - } - try(t, cp, false) - }) -} - var _ ConsulACLsAPI = (*consulACLsAPI)(nil) var _ ConsulACLsAPI = (*mockConsulACLsAPI)(nil) @@ -229,15 +148,15 @@ func TestConsulACLsAPI_CheckSIPolicy(t *testing.T) { } t.Run("operator has service write", func(t *testing.T) { - try(t, "service1", consul.ExampleOperatorToken1, "") + try(t, "service1", consul.ExampleOperatorTokenID1, "") }) t.Run("operator has service_prefix write", func(t *testing.T) { - try(t, "foo-service1", consul.ExampleOperatorToken2, "") + try(t, "foo-service1", consul.ExampleOperatorTokenID2, "") }) t.Run("operator permissions insufficient", func(t *testing.T) { - try(t, "service1", consul.ExampleOperatorToken3, + try(t, "service1", consul.ExampleOperatorTokenID3, "permission denied for \"service1\"", ) }) @@ -252,88 +171,3 @@ func TestConsulACLsAPI_CheckSIPolicy(t *testing.T) { ) }) } - -func TestConsulACLsAPI_allowsServiceWrite(t *testing.T) { - t.Parallel() - - try := func(t *testing.T, task string, cp *ConsulPolicy, exp bool) { - cAPI := new(consulACLsAPI) - result := cAPI.allowsServiceWrite(task, cp) - require.Equal(t, exp, result) - } - - makeCP := func(services [][2]string, prefixes [][2]string) *ConsulPolicy { - serviceRules := make([]*ConsulServiceRule, 0, len(services)) - for _, service := range services { - serviceRules = append(serviceRules, &ConsulServiceRule{Name: service[0], Policy: service[1]}) - } - prefixRules := make([]*ConsulServiceRule, 0, len(prefixes)) - for _, prefix := range prefixes { - prefixRules = append(prefixRules, &ConsulServiceRule{Name: prefix[0], Policy: prefix[1]}) - } - return &ConsulPolicy{Services: serviceRules, ServicePrefixes: prefixRules} - } - - t.Run("matching service policy write", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"task1", "write"}}, - nil, - ), true) - }) - - t.Run("matching service policy read", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"task1", "read"}}, - nil, - ), false) - }) - - t.Run("wildcard service policy write", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"*", "write"}}, - nil, - ), true) - }) - - t.Run("wrong service policy write", func(t *testing.T) { - try(t, "other1", makeCP( - [][2]string{{"task1", "write"}}, - nil, - ), false) - }) - - t.Run("matching prefix policy write", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"task-", "write"}}, - ), true) - }) - - t.Run("matching prefix policy read", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"task-", "read"}}, - ), false) - }) - - t.Run("empty prefix policy write", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"", "write"}}, - ), true) - }) - - t.Run("late matching service", func(t *testing.T) { - try(t, "task1", makeCP( - [][2]string{{"task0", "write"}, {"task1", "write"}}, - nil, - ), true) - }) - - t.Run("late matching prefix", func(t *testing.T) { - try(t, "task-one", makeCP( - nil, - [][2]string{{"foo-", "write"}, {"task-", "write"}}, - ), true) - }) -} diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 257961fca..9889b2f32 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -320,8 +320,8 @@ func TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse(t *testing.T) { // Each variation of the provided Consul operator token noOpToken := "" unrecognizedOpToken := uuid.Generate() - unauthorizedOpToken := consul.ExampleOperatorToken3 - authorizedOpToken := consul.ExampleOperatorToken1 + unauthorizedOpToken := consul.ExampleOperatorTokenID3 + authorizedOpToken := consul.ExampleOperatorTokenID1 t.Run("no token provided", func(t *testing.T) { request := newRequest(job) diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 4011dc421..c92d1b6f5 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -1714,6 +1714,8 @@ func (n *Node) DeriveSIToken(args *structs.DeriveSITokenRequest, reply *structs. } // make sure each task in args.Tasks is a connect-enabled task + // note: the tasks at this point should be the "connect-sidecar-" name + // unneeded := tasksNotUsingConnect(tg, args.Tasks) if len(unneeded) > 0 { setError(fmt.Errorf( @@ -1751,6 +1753,7 @@ func (n *Node) DeriveSIToken(args *structs.DeriveSITokenRequest, reply *structs. if !ok { return nil } + sii := ServiceIdentityIndex{ ClusterID: clusterID, AllocID: alloc.ID,