Skip to content

Commit

Permalink
always update acl policy if it exists
Browse files Browse the repository at this point in the history
  • Loading branch information
aahel committed Jun 26, 2023
1 parent 79db263 commit 9a8c45b
Showing 1 changed file with 26 additions and 31 deletions.
57 changes: 26 additions & 31 deletions control-plane/subcommand/server-acl-init/create_or_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,42 +315,37 @@ func (c *Command) createOrUpdateACLPolicy(policy api.ACLPolicy, consulClient *ap
// Allowing the Consul node name to be configurable also requires any sync
// policy to be updated in case the node name has changed.
if isPolicyExistsErr(err, policy.Name) {
if c.flagEnableNamespaces || c.flagSyncCatalog {
c.log.Info(fmt.Sprintf("Policy %q already exists, updating", policy.Name))
c.log.Info(fmt.Sprintf("Policy %q already exists, updating", policy.Name))

// The policy ID is required in any PolicyUpdate call, so first we need to
// get the existing policy to extract its ID.
existingPolicies, _, err := consulClient.ACL().PolicyList(&api.QueryOptions{})
if err != nil {
return err
}

// Find the policy that matches our name and description
// and that's the ID we need
for _, existingPolicy := range existingPolicies {
if existingPolicy.Name == policy.Name && existingPolicy.Description == policy.Description {
policy.ID = existingPolicy.ID
}
}
// The policy ID is required in any PolicyUpdate call, so first we need to
// get the existing policy to extract its ID.
existingPolicies, _, err := consulClient.ACL().PolicyList(&api.QueryOptions{})
if err != nil {
return err
}

// This shouldn't happen, because we're looking for a policy
// only after we've hit a `Policy already exists` error.
// The only time it might happen is if a user has manually created a policy
// with this name but used a different description. In this case,
// we don't want to overwrite the policy so we just error.
if policy.ID == "" {
return fmt.Errorf("policy found with name %q but not with expected description %q; "+
"if this policy was created manually it must be renamed to something else because this name is reserved by consul-k8s",
policy.Name, policy.Description)
// Find the policy that matches our name and description
// and that's the ID we need
for _, existingPolicy := range existingPolicies {
if existingPolicy.Name == policy.Name && existingPolicy.Description == policy.Description {
policy.ID = existingPolicy.ID
}
}

// Update the policy now that we've found its ID
_, _, err = consulClient.ACL().PolicyUpdate(&policy, &api.WriteOptions{})
return err
} else {
c.log.Info(fmt.Sprintf("Policy %q already exists, skipping update", policy.Name))
return nil
// This shouldn't happen, because we're looking for a policy
// only after we've hit a `Policy already exists` error.
// The only time it might happen is if a user has manually created a policy
// with this name but used a different description. In this case,
// we don't want to overwrite the policy so we just error.
if policy.ID == "" {
return fmt.Errorf("policy found with name %q but not with expected description %q; "+
"if this policy was created manually it must be renamed to something else because this name is reserved by consul-k8s",
policy.Name, policy.Description)
}

// Update the policy now that we've found its ID
_, _, err = consulClient.ACL().PolicyUpdate(&policy, &api.WriteOptions{})
return err
}
return err
}
Expand Down

0 comments on commit 9a8c45b

Please sign in to comment.