Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Net 12039 terminating gateway acl policy fix #4468

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/4468.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
control-plane: Fixed bug in TerminatingGateway controller workflow for handling AdminPartition enabled cluster ACL policies for associated TerminatingGateway services ([NET-12039](https://hashicorp.atlassian.net/browse/NET-12039)).
```
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ type ConfigEntryController struct {
// `k8s-default` namespace.
NSMirroringPrefix string

// ConsulPartition indicates the Consul Admin Partition name the controller is
// operating in. Adds this value as metadata on managed resources.
ConsulPartition string

// CrossNSACLPolicy is the name of the ACL policy to attach to
// any created Consul namespaces to allow cross namespace service discovery.
// Only necessary if ACLs are enabled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type TerminatingGatewayController struct {
FinalizerPatcher

NamespacesEnabled bool
PartitionsEnabled bool

Log logr.Logger
Scheme *runtime.Scheme
Expand All @@ -49,33 +50,49 @@ func init() {

type templateArgs struct {
Namespace string
Partition string
ServiceName string
EnableNamespaces bool
EnablePartitions bool
}

var (
servicePolicyTpl *template.Template
servicePolicyRulesTpl = `
{{- if .EnablePartitions }}
partition "{{.Partition}}" {
{{- end }}
{{- if .EnableNamespaces }}
namespace "{{.Namespace}}" {
namespace "{{.Namespace}}" {
{{- end }}
service "{{.ServiceName}}" {
policy = "write"
}
service "{{.ServiceName}}" {
policy = "write"
intention = "read"
}
{{- if .EnableNamespaces }}
}
{{- end }}
{{- if .EnablePartitions }}
}
{{- end }}
`

wildcardPolicyTpl *template.Template
wildcardPolicyRulesTpl = `
{{- if .EnablePartitions }}
partition "{{.Partition}}" {
{{- end }}
{{- if .EnableNamespaces }}
namespace "{{.Namespace}}" {
namespace "{{.Namespace}}" {
{{- end }}
service_prefix "" {
policy = "write"
}
service_prefix "" {
policy = "write"
intention = "read"
}
{{- if .EnableNamespaces }}
}
{{- end }}
{{- if .EnablePartitions }}
}
{{- end }}
`
Expand Down Expand Up @@ -104,7 +121,7 @@ func (r *TerminatingGatewayController) Reconcile(ctx context.Context, req ctrl.R
}

if enabled {
err := r.updateACls(log, termGW)
err = r.updateACls(log, termGW)
if err != nil {
log.Error(err, "error updating terminating-gateway roles")
r.UpdateStatusFailedToSetACLs(ctx, termGW, err)
Expand Down Expand Up @@ -165,13 +182,17 @@ func (r *TerminatingGatewayController) aclsEnabled() (bool, error) {
return state.Token != "", nil
}

func (r *TerminatingGatewayController) adminPartition() string {
return defaultIfEmpty(r.ConfigEntryController.ConsulPartition)
}

func (r *TerminatingGatewayController) updateACls(log logr.Logger, termGW *consulv1alpha1.TerminatingGateway) error {
client, err := consul.NewClientFromConnMgr(r.ConfigEntryController.ConsulClientConfig, r.ConfigEntryController.ConsulServerConnMgr)
connMgrClient, err := consul.NewClientFromConnMgr(r.ConfigEntryController.ConsulClientConfig, r.ConfigEntryController.ConsulServerConnMgr)
if err != nil {
return err
}

roles, _, err := client.ACL().RoleList(nil)
roles, _, err := connMgrClient.ACL().RoleList(nil)
if err != nil {
return err
}
Expand All @@ -189,7 +210,7 @@ func (r *TerminatingGatewayController) updateACls(log logr.Logger, termGW *consu
return errors.New("terminating gateway role not found")
}

terminatingGatewayRole, _, err := client.ACL().RoleRead(terminatingGatewayRoleID, nil)
terminatingGatewayRole, _, err := connMgrClient.ACL().RoleRead(terminatingGatewayRoleID, nil)
if err != nil {
return err
}
Expand All @@ -214,7 +235,7 @@ func (r *TerminatingGatewayController) updateACls(log logr.Logger, termGW *consu
}

if termGW.ObjectMeta.DeletionTimestamp.IsZero() {
termGWPoliciesToKeep, termGWPoliciesToRemove, err = r.handleModificationForPolicies(log, client, existingTermGWPolicies, termGW.Spec.Services)
termGWPoliciesToKeep, termGWPoliciesToRemove, err = r.handleModificationForPolicies(log, connMgrClient, existingTermGWPolicies, termGW.Spec.Services)
if err != nil {
return err
}
Expand All @@ -225,12 +246,12 @@ func (r *TerminatingGatewayController) updateACls(log logr.Logger, termGW *consu
termGWPoliciesToKeep = append(termGWPoliciesToKeep, terminatingGatewayPolicy)
terminatingGatewayRole.Policies = termGWPoliciesToKeep

_, _, err = client.ACL().RoleUpdate(terminatingGatewayRole, nil)
_, _, err = connMgrClient.ACL().RoleUpdate(terminatingGatewayRole, nil)
if err != nil {
return err
}

err = r.conditionallyDeletePolicies(log, client, termGWPoliciesToRemove, termGW.Name)
err = r.conditionallyDeletePolicies(log, connMgrClient, termGWPoliciesToRemove, termGW.Name)
if err != nil {
return err
}
Expand All @@ -253,18 +274,29 @@ func (r *TerminatingGatewayController) handleModificationForPolicies(log logr.Lo

termGWPoliciesToKeepNames := mapset.NewSet[string]()
for _, service := range services {
log.Info("Checking for existing policies", "policy", servicePolicyName(service.Name, defaultIfEmpty(service.Namespace)))
existingPolicy, _, err := client.ACL().PolicyReadByName(servicePolicyName(service.Name, defaultIfEmpty(service.Namespace)), &capi.QueryOptions{})
if err != nil {
log.Error(err, "error reading policy")
return nil, nil, err
}
if existingPolicy != nil {
log.Info("Found for existing policies", "policy", existingPolicy.Name, "ID", existingPolicy.ID)
} else {
log.Info("Did not find for existing policies", "policy", servicePolicyName(service.Name, defaultIfEmpty(service.Namespace)))
}

if existingPolicy == nil {
policyTemplate := getPolicyTemplateFor(service.Name)
policyNamespace := defaultIfEmpty(service.Namespace)
policyAdminPartition := r.adminPartition()
log.Info("Templating new ACL Policy", "Service", service.Name, "Namespace", policyNamespace, "Partition", policyAdminPartition)
var data bytes.Buffer
if err := policyTemplate.Execute(&data, templateArgs{
EnableNamespaces: r.NamespacesEnabled,
Namespace: defaultIfEmpty(service.Namespace),
EnablePartitions: r.PartitionsEnabled,
Namespace: policyNamespace,
Partition: policyAdminPartition,
ServiceName: service.Name,
}); err != nil {
// just panic if we can't compile the simple template
Expand All @@ -277,7 +309,10 @@ func (r *TerminatingGatewayController) handleModificationForPolicies(log logr.Lo
Rules: data.String(),
}, nil)
if err != nil {
log.Error(err, "error creating policy")
return nil, nil, err
} else {
log.Info("Created new ACL Policy", "Service", service.Name, "Namespace", policyNamespace, "Partition", policyAdminPartition)
}
}

Expand Down
2 changes: 2 additions & 0 deletions control-plane/subcommand/inject-connect/v1controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ func (c *Command) configureControllers(ctx context.Context, mgr manager.Manager,
ConsulDestinationNamespace: c.flagConsulDestinationNamespace,
EnableNSMirroring: c.flagEnableK8SNSMirroring,
NSMirroringPrefix: c.flagK8SNSMirroringPrefix,
ConsulPartition: c.consul.Partition,
CrossNSACLPolicy: c.flagCrossNamespaceACLPolicy,
}
if err := (&controllers.ServiceDefaultsController{
Expand Down Expand Up @@ -275,6 +276,7 @@ func (c *Command) configureControllers(ctx context.Context, mgr manager.Manager,
Client: mgr.GetClient(),
Log: ctrl.Log.WithName("controller").WithName(apicommon.TerminatingGateway),
NamespacesEnabled: c.flagEnableNamespaces,
PartitionsEnabled: c.flagEnablePartitions,
Scheme: mgr.GetScheme(),
}).SetupWithManager(ctx, mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", apicommon.TerminatingGateway)
Expand Down
Loading