From be5636dcfa8b8d84cb87a0f8a21697dc96768c66 Mon Sep 17 00:00:00 2001 From: natemollica-dev Date: Wed, 22 Jan 2025 11:59:08 -0800 Subject: [PATCH 1/4] Partition working for single cluster default partition for wildcard and individual services --- .../configentries/configentry_controller.go | 4 ++ .../terminatinggateway_controller.go | 51 ++++++++++++++----- .../inject-connect/v1controllers.go | 2 + 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/control-plane/controllers/configentries/configentry_controller.go b/control-plane/controllers/configentries/configentry_controller.go index dc68aea619..886ed3f435 100644 --- a/control-plane/controllers/configentries/configentry_controller.go +++ b/control-plane/controllers/configentries/configentry_controller.go @@ -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. diff --git a/control-plane/controllers/configentries/terminatinggateway_controller.go b/control-plane/controllers/configentries/terminatinggateway_controller.go index ec329bd17c..1dd2250997 100644 --- a/control-plane/controllers/configentries/terminatinggateway_controller.go +++ b/control-plane/controllers/configentries/terminatinggateway_controller.go @@ -36,6 +36,7 @@ type TerminatingGatewayController struct { FinalizerPatcher NamespacesEnabled bool + PartitionsEnabled bool Log logr.Logger Scheme *runtime.Scheme @@ -49,33 +50,45 @@ func init() { type templateArgs struct { Namespace string + Partition string ServiceName string EnableNamespaces bool + EnablePartitions bool } var ( servicePolicyTpl *template.Template servicePolicyRulesTpl = ` +{{- if .EnablePartitions }} +partition "{{.Partition}}" { {{- if .EnableNamespaces }} -namespace "{{.Namespace}}" { + namespace "{{.Namespace}}" { {{- end }} - service "{{.ServiceName}}" { - policy = "write" - } + service "{{.ServiceName}}" { + policy = "write" + intention = "read" + } {{- if .EnableNamespaces }} + } +{{- end }} } {{- end }} ` wildcardPolicyTpl *template.Template wildcardPolicyRulesTpl = ` +{{- if .EnablePartitions }} +partition "{{.Partition}}" { {{- if .EnableNamespaces }} -namespace "{{.Namespace}}" { + namespace "{{.Namespace}}" { {{- end }} - service_prefix "" { - policy = "write" - } + service_prefix "" { + policy = "write" + intention = "read" + } {{- if .EnableNamespaces }} + } +{{- end }} } {{- end }} ` @@ -165,13 +178,21 @@ func (r *TerminatingGatewayController) aclsEnabled() (bool, error) { return state.Token != "", nil } +func (r *TerminatingGatewayController) partitionsEnabled() (bool, error) { + state, err := r.ConfigEntryController.ConsulServerConnMgr.State() + if err != nil { + return false, err + } + return state.Token != "", nil +} + 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 } @@ -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 } @@ -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 } @@ -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 } @@ -264,7 +285,9 @@ func (r *TerminatingGatewayController) handleModificationForPolicies(log logr.Lo var data bytes.Buffer if err := policyTemplate.Execute(&data, templateArgs{ EnableNamespaces: r.NamespacesEnabled, + EnablePartitions: r.PartitionsEnabled, Namespace: defaultIfEmpty(service.Namespace), + Partition: defaultIfEmpty(r.ConfigEntryController.ConsulPartition), ServiceName: service.Name, }); err != nil { // just panic if we can't compile the simple template diff --git a/control-plane/subcommand/inject-connect/v1controllers.go b/control-plane/subcommand/inject-connect/v1controllers.go index dd6b3be739..e5c3ae2b06 100644 --- a/control-plane/subcommand/inject-connect/v1controllers.go +++ b/control-plane/subcommand/inject-connect/v1controllers.go @@ -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{ @@ -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) From 72c7ddd5ce04e5dee23207d55e9e538e32b9877f Mon Sep 17 00:00:00 2001 From: natemollica-dev Date: Wed, 22 Jan 2025 14:59:19 -0800 Subject: [PATCH 2/4] Partition template update for non-AP deployments --- .../configentries/terminatinggateway_controller.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/control-plane/controllers/configentries/terminatinggateway_controller.go b/control-plane/controllers/configentries/terminatinggateway_controller.go index 1dd2250997..4232ed2134 100644 --- a/control-plane/controllers/configentries/terminatinggateway_controller.go +++ b/control-plane/controllers/configentries/terminatinggateway_controller.go @@ -61,6 +61,7 @@ var ( servicePolicyRulesTpl = ` {{- if .EnablePartitions }} partition "{{.Partition}}" { +{{- end }} {{- if .EnableNamespaces }} namespace "{{.Namespace}}" { {{- end }} @@ -71,6 +72,7 @@ partition "{{.Partition}}" { {{- if .EnableNamespaces }} } {{- end }} +{{- if .EnablePartitions }} } {{- end }} ` @@ -79,6 +81,7 @@ partition "{{.Partition}}" { wildcardPolicyRulesTpl = ` {{- if .EnablePartitions }} partition "{{.Partition}}" { +{{- end }} {{- if .EnableNamespaces }} namespace "{{.Namespace}}" { {{- end }} @@ -89,6 +92,7 @@ partition "{{.Partition}}" { {{- if .EnableNamespaces }} } {{- end }} +{{- if .EnablePartitions }} } {{- end }} ` From d86daf7bd86199e86d7a633967ec0fb6c2d19d45 Mon Sep 17 00:00:00 2001 From: natemollica-dev Date: Fri, 24 Jan 2025 15:23:01 -0800 Subject: [PATCH 3/4] Partition logging additions and default partition fallback --- .../terminatinggateway_controller.go | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/control-plane/controllers/configentries/terminatinggateway_controller.go b/control-plane/controllers/configentries/terminatinggateway_controller.go index 4232ed2134..2eb18c0848 100644 --- a/control-plane/controllers/configentries/terminatinggateway_controller.go +++ b/control-plane/controllers/configentries/terminatinggateway_controller.go @@ -121,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) @@ -182,12 +182,8 @@ func (r *TerminatingGatewayController) aclsEnabled() (bool, error) { return state.Token != "", nil } -func (r *TerminatingGatewayController) partitionsEnabled() (bool, error) { - state, err := r.ConfigEntryController.ConsulServerConnMgr.State() - if err != nil { - return false, err - } - 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 { @@ -278,20 +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, EnablePartitions: r.PartitionsEnabled, - Namespace: defaultIfEmpty(service.Namespace), - Partition: defaultIfEmpty(r.ConfigEntryController.ConsulPartition), + Namespace: policyNamespace, + Partition: policyAdminPartition, ServiceName: service.Name, }); err != nil { // just panic if we can't compile the simple template @@ -304,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) } } From 78ca1931a4bcb827515e6d95dcd20c1a9f6bf83e Mon Sep 17 00:00:00 2001 From: natemollica-dev Date: Fri, 24 Jan 2025 15:36:04 -0800 Subject: [PATCH 4/4] Changelog add --- .changelog/4468.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/4468.txt diff --git a/.changelog/4468.txt b/.changelog/4468.txt new file mode 100644 index 0000000000..2e28cf4656 --- /dev/null +++ b/.changelog/4468.txt @@ -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)). +```