Skip to content

Commit

Permalink
backport of commit e4fdccd (#15936)
Browse files Browse the repository at this point in the history
This pull request was automerged via backport-assistant
  • Loading branch information
hc-github-team-nomad-core authored Jan 28, 2023
1 parent 74bd098 commit 7ea0baf
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changelog/15928.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
consul: Fixed a bug where acceptable service identity on Consul token was not accepted
```
11 changes: 11 additions & 0 deletions command/agent/consul/acl_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ const (
ExampleOperatorTokenID3 = "6177d1b9-c0f6-4118-b891-d818a3cb80b1"
ExampleOperatorTokenID4 = "754ae26c-f3cc-e088-d486-9c0d20f5eaea"
ExampleOperatorTokenID5 = "097cbb45-506b-c79c-ec38-82eb0dc0794a"
ExampleOperatorTokenID6 = "6268bd42-6f72-4c90-9c83-90ed6336dcf9"
)

// Example Consul ACL tokens for use in tests that match the policies as the
Expand Down Expand Up @@ -214,6 +215,16 @@ var (
Namespace: "",
}

ExampleOperatorToken6 = &api.ACLToken{
SecretID: ExampleOperatorTokenID6,
AccessorID: "93786935-8856-6e17-0488-c5370a1f044e",
Description: "Operator Token 6",
ServiceIdentities: []*api.ACLServiceIdentity{
{ServiceName: "service1"},
},
Namespace: "",
}

// In Consul namespace "banana"

ExampleOperatorToken10 = &api.ACLToken{
Expand Down
8 changes: 8 additions & 0 deletions nomad/consul_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,14 @@ func (c *consulACLsAPI) canWriteService(namespace, service string, token *api.AC
// treat that like an exact match to preserve backwards compatibility
matches := (namespace == token.Namespace) || (namespace == "" && token.Namespace == "default")

// check each service identity attached to the token -
// the virtual policy for service identities enables service:write
for _, si := range token.ServiceIdentities {
if si.ServiceName == service {
return true, nil
}
}

// check each policy directly attached to the token
for _, policyRef := range token.Policies {
if allowable, err := c.policyAllowsServiceWrite(matches, namespace, service, policyRef.ID); err != nil {
Expand Down
11 changes: 7 additions & 4 deletions nomad/consul_policy_oss_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build !ent
// +build !ent

package nomad

Expand All @@ -10,7 +9,7 @@ import (
"github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/command/agent/consul"
"github.com/hashicorp/nomad/helper/testlog"
"github.com/stretchr/testify/require"
"github.com/shoenig/test/must"
)

func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
Expand All @@ -23,8 +22,8 @@ func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
logger: logger,
}
result, err := cAPI.canWriteService(namespace, task, token)
require.NoError(t, err)
require.Equal(t, exp, result)
must.NoError(t, err)
must.Eq(t, exp, result)
}

// In Nomad OSS, group consul namespace will always be empty string.
Expand All @@ -41,5 +40,9 @@ func TestConsulACLsAPI_hasSufficientPolicy_oss(t *testing.T) {
t.Run("working role only", func(t *testing.T) {
try(t, "", "service1", consul.ExampleOperatorToken4, true)
})

t.Run("working service identity only", func(t *testing.T) {
try(t, "", "service1", consul.ExampleOperatorToken6, true)
})
})
}

0 comments on commit 7ea0baf

Please sign in to comment.