diff --git a/api/v1alpha1/servicedefaults_types.go b/api/v1alpha1/servicedefaults_types.go index 328314d968..970fc4aaaf 100644 --- a/api/v1alpha1/servicedefaults_types.go +++ b/api/v1alpha1/servicedefaults_types.go @@ -54,9 +54,8 @@ func init() { // ToConsul converts the entry into it's Consul equivalent struct. func (s *ServiceDefaults) ToConsul() *capi.ServiceConfigEntry { return &capi.ServiceConfigEntry{ - Kind: capi.ServiceDefaults, - Name: s.Name, - //Namespace: s.Namespace, // todo: don't set this unless enterprise + Kind: capi.ServiceDefaults, + Name: s.Name, Protocol: s.Spec.Protocol, MeshGateway: s.Spec.MeshGateway.toConsul(), Expose: s.Spec.Expose.toConsul(), diff --git a/api/v1alpha1/servicedefaults_webhook_test.go b/api/v1alpha1/servicedefaults_webhook_test.go index f14c68ae67..2b7855f967 100644 --- a/api/v1alpha1/servicedefaults_webhook_test.go +++ b/api/v1alpha1/servicedefaults_webhook_test.go @@ -45,7 +45,7 @@ func TestRun_HandleErrorsIfServiceDefaultsWithSameNameExists(t *testing.T) { validator := &serviceDefaultsValidator{ Client: client, ConsulClient: consulClient, - Logger: logrtest.NullLogger{}, + Logger: logrtest.TestLogger{T: t}, } decoder, err := admission.NewDecoder(scheme.Scheme) diff --git a/catalog/to-consul/syncer.go b/catalog/to-consul/syncer.go index d51282cb33..5a45f99005 100644 --- a/catalog/to-consul/syncer.go +++ b/catalog/to-consul/syncer.go @@ -7,6 +7,7 @@ import ( "github.com/cenkalti/backoff" "github.com/deckarep/golang-set" + "github.com/hashicorp/consul-k8s/namespaces" "github.com/hashicorp/consul/api" "github.com/hashicorp/go-hclog" ) @@ -417,9 +418,7 @@ func (s *ConsulSyncer) syncFull(ctx context.Context) { for _, services := range s.namespaces { for _, r := range services { if s.EnableNamespaces { - // Check and potentially create the service's namespace if - // it doesn't already exist - err := s.checkAndCreateNamespace(r.Service.Namespace) + err := namespaces.EnsureExists(s.Client, r.Service.Namespace, s.CrossNamespaceACLPolicy) if err != nil { s.Log.Warn("error checking and creating Consul namespace", "node-name", r.Node, @@ -475,40 +474,3 @@ func (s *ConsulSyncer) init() { s.initialSync = make(chan bool) } } - -func (s *ConsulSyncer) checkAndCreateNamespace(ns string) error { - // Check if the Consul namespace exists - namespaceInfo, _, err := s.Client.Namespaces().Read(ns, nil) - if err != nil { - return err - } - - // If not, create it - if namespaceInfo == nil { - var aclConfig api.NamespaceACLConfig - if s.CrossNamespaceACLPolicy != "" { - // Create the ACLs config for the cross-Consul-namespace - // default policy that needs to be attached - aclConfig = api.NamespaceACLConfig{ - PolicyDefaults: []api.ACLLink{ - {Name: s.CrossNamespaceACLPolicy}, - }, - } - } - - consulNamespace := api.Namespace{ - Name: ns, - Description: "Auto-generated by a Catalog Sync Process", - ACLs: &aclConfig, - Meta: map[string]string{"external-source": "kubernetes"}, - } - - _, _, err = s.Client.Namespaces().Create(&consulNamespace, nil) - if err != nil { - return err - } - s.Log.Info("creating consul namespace", "name", consulNamespace.Name) - } - - return nil -} diff --git a/connect-inject/handler.go b/connect-inject/handler.go index ccc1c162df..377669ebbc 100644 --- a/connect-inject/handler.go +++ b/connect-inject/handler.go @@ -9,6 +9,7 @@ import ( "strconv" "github.com/deckarep/golang-set" + "github.com/hashicorp/consul-k8s/namespaces" "github.com/hashicorp/consul/api" "github.com/hashicorp/go-hclog" "github.com/mattbaird/jsonpatch" @@ -370,8 +371,7 @@ func (h *Handler) Mutate(req *v1beta1.AdmissionRequest) *v1beta1.AdmissionRespon // all patches are created to guarantee no errors were encountered in // that process before modifying the Consul cluster. if h.EnableNamespaces { - // Check if the namespace exists. If not, create it. - if err := h.checkAndCreateNamespace(h.consulNamespace(req.Namespace)); err != nil { + if err := namespaces.EnsureExists(h.ConsulClient, h.consulNamespace(req.Namespace), h.CrossNamespaceACLPolicy); err != nil { h.Log.Error("Error checking or creating namespace", "err", err, "Namespace", h.consulNamespace(req.Namespace), "Request Name", req.Name) return &v1beta1.AdmissionResponse{ @@ -503,42 +503,6 @@ func (h *Handler) consulNamespace(ns string) string { } } -func (h *Handler) checkAndCreateNamespace(ns string) error { - // Check if the Consul namespace exists - namespaceInfo, _, err := h.ConsulClient.Namespaces().Read(ns, nil) - if err != nil { - return err - } - - // If not, create it - if namespaceInfo == nil { - var aclConfig api.NamespaceACLConfig - if h.CrossNamespaceACLPolicy != "" { - // Create the ACLs config for the cross-Consul-namespace - // default policy that needs to be attached - aclConfig = api.NamespaceACLConfig{ - PolicyDefaults: []api.ACLLink{ - {Name: h.CrossNamespaceACLPolicy}, - }, - } - } - - consulNamespace := api.Namespace{ - Name: ns, - Description: "Auto-generated by a Connect Injector", - ACLs: &aclConfig, - Meta: map[string]string{"external-source": "kubernetes"}, - } - - _, _, err = h.ConsulClient.Namespaces().Create(&consulNamespace, nil) - if err != nil { - return err - } - } - - return nil -} - func portValue(pod *corev1.Pod, value string) (int32, error) { // First search for the named port for _, c := range pod.Spec.Containers { diff --git a/connect-inject/handler_ent_test.go b/connect-inject/handler_ent_test.go index 11fabf90bb..3779d21edf 100644 --- a/connect-inject/handler_ent_test.go +++ b/connect-inject/handler_ent_test.go @@ -225,7 +225,7 @@ func TestHandler_MutateWithNamespaces(t *testing.T) { // Check created namespace properties if ns != "default" { - require.Equalf("Auto-generated by a Connect Injector", actNamespace.Description, + require.Equalf("Auto-generated by consul-k8s", actNamespace.Description, "wrong namespace description for namespace %s", ns) require.Containsf(actNamespace.Meta, "external-source", "namespace %s does not contain external-source metadata key", ns) @@ -420,7 +420,6 @@ func TestHandler_MutateWithNamespaces_ACLs(t *testing.T) { a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.ACL.Enabled = true }) - require.NoError(t, err) defer a.Stop() // Set up a client for bootstrapping @@ -489,7 +488,7 @@ func TestHandler_MutateWithNamespaces_ACLs(t *testing.T) { // Check created namespace properties if ns != "default" { - require.Equalf(t, "Auto-generated by a Connect Injector", actNamespace.Description, + require.Equalf(t, "Auto-generated by consul-k8s", actNamespace.Description, "wrong namespace description for namespace %s", ns) require.Containsf(t, actNamespace.Meta, "external-source", "namespace %s does not contain external-source metadata key", ns) diff --git a/controllers/servicedefaults_controller.go b/controllers/servicedefaults_controller.go index 5ea9380d7f..406305f014 100644 --- a/controllers/servicedefaults_controller.go +++ b/controllers/servicedefaults_controller.go @@ -2,10 +2,11 @@ package controllers import ( "context" - "errors" + "fmt" "strings" "github.com/go-logr/logr" + "github.com/hashicorp/consul-k8s/namespaces" capi "github.com/hashicorp/consul/api" corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" @@ -29,6 +30,30 @@ type ServiceDefaultsReconciler struct { Log logr.Logger Scheme *runtime.Scheme ConsulClient *capi.Client + + // EnableConsulNamespaces indicates that a user is running Consul Enterprise + // with version 1.7+ which supports namespaces. + EnableConsulNamespaces bool + + // ConsulDestinationNamespace is the name of the Consul namespace to create + // all config entries in. If EnableNSMirroring is true this is ignored. + ConsulDestinationNamespace string + + // EnableNSMirroring causes Consul namespaces to be created to match the + // k8s namespace of any config entry custom resource. Config entries will + // be created in the matching Consul namespace. + EnableNSMirroring bool + + // NSMirroringPrefix is an optional prefix that can be added to the Consul + // namespaces created while mirroring. For example, if it is set to "k8s-", + // then the k8s `default` namespace will be mirrored in Consul's + // `k8s-default` namespace. + NSMirroringPrefix 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. + CrossNSACLPolicy string } // +kubebuilder:rbac:groups=consul.hashicorp.com,resources=servicedefaults,verbs=get;list;watch;create;update;patch;delete @@ -36,14 +61,14 @@ type ServiceDefaultsReconciler struct { func (r *ServiceDefaultsReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { ctx := context.Background() - logger := r.Log.WithValues("servicedefaults", req.NamespacedName) + logger := r.Log.WithValues("controller", "servicedefaults", "request", req.NamespacedName) var svcDefaults consulv1alpha1.ServiceDefaults err := r.Get(ctx, req.NamespacedName, &svcDefaults) if k8serr.IsNotFound(err) { return ctrl.Result{}, client.IgnoreNotFound(err) } else if err != nil { - logger.Error(err, "failed to retrieve resource") + logger.Error(err, "retrieving resource") return ctrl.Result{}, err } @@ -64,9 +89,11 @@ func (r *ServiceDefaultsReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er logger.Info("deletion event") // Our finalizer is present, so we need to delete the config entry // from consul. - _, err = r.ConsulClient.ConfigEntries().Delete(capi.ServiceDefaults, svcDefaults.Name, nil) + _, err = r.ConsulClient.ConfigEntries().Delete(capi.ServiceDefaults, svcDefaults.Name, &capi.WriteOptions{ + Namespace: r.consulNamespace(req.Namespace), + }) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, fmt.Errorf("deleting config entry from consul: %w", err) } logger.Info("deletion from Consul successful") @@ -83,61 +110,55 @@ func (r *ServiceDefaultsReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er } // Check to see if consul has service defaults with the same name - entry, _, err := r.ConsulClient.ConfigEntries().Get(capi.ServiceDefaults, svcDefaults.Name, nil) + entry, _, err := r.ConsulClient.ConfigEntries().Get(capi.ServiceDefaults, svcDefaults.Name, &capi.QueryOptions{ + Namespace: r.consulNamespace(req.Namespace), + }) // If a config entry with this name does not exist if isNotFoundErr(err) { - // Create the config entry - _, _, err := r.ConsulClient.ConfigEntries().Set(svcDefaults.ToConsul(), nil) - if err != nil { - svcDefaults.Status.Conditions = syncFailed(ConsulAgentError, err.Error()) - if err := r.Status().Update(context.Background(), &svcDefaults); err != nil { - return ctrl.Result{}, err + logger.Info("config entry not found in consul") + + // If Consul namespaces are enabled we may need to create the + // destination consul namespace first. + if r.EnableConsulNamespaces { + if err := namespaces.EnsureExists(r.ConsulClient, r.consulNamespace(req.Namespace), r.CrossNSACLPolicy); err != nil { + return r.syncFailed(logger, svcDefaults, ConsulAgentError, + fmt.Errorf("creating consul namespace %q: %w", r.consulNamespace(req.Namespace), err)) } - return ctrl.Result{}, err } - svcDefaults.Status.Conditions = syncSuccessful() - if err := r.Status().Update(context.Background(), &svcDefaults); err != nil { - return ctrl.Result{}, err + + // Create the config entry + _, _, err := r.ConsulClient.ConfigEntries().Set(svcDefaults.ToConsul(), &capi.WriteOptions{ + Namespace: r.consulNamespace(req.Namespace), + }) + if err != nil { + return r.syncFailed(logger, svcDefaults, ConsulAgentError, + fmt.Errorf("writing config entry to consul: %w", err)) } - return ctrl.Result{}, nil + return r.syncSuccessful(svcDefaults) } - // If there is an error when trying to get the config entry from the api server, fail the reconcile + // If there is an error when trying to get the config entry from the api server, + // fail the reconcile. if err != nil { - svcDefaults.Status.Conditions = syncFailed(ConsulAgentError, err.Error()) - if err := r.Status().Update(context.Background(), &svcDefaults); err != nil { - return ctrl.Result{}, err - } - return ctrl.Result{}, err + return r.syncFailed(logger, svcDefaults, ConsulAgentError, err) } svcDefaultEntry, ok := entry.(*capi.ServiceConfigEntry) if !ok { - err := errors.New("could not cast entry as ServiceConfigEntry") - svcDefaults.Status.Conditions = syncUnknownWithError(CastError, err.Error()) - if err := r.Status().Update(context.Background(), &svcDefaults); err != nil { - return ctrl.Result{}, err - } - return ctrl.Result{}, err + return r.syncUnknownWithError(logger, svcDefaults, CastError, + fmt.Errorf("could not cast entry as ServiceConfigEntry")) } if !svcDefaults.MatchesConsul(svcDefaultEntry) { - _, _, err := r.ConsulClient.ConfigEntries().Set(svcDefaults.ToConsul(), nil) + _, _, err := r.ConsulClient.ConfigEntries().Set(svcDefaults.ToConsul(), &capi.WriteOptions{ + Namespace: r.consulNamespace(req.Namespace), + }) if err != nil { - svcDefaults.Status.Conditions = syncUnknownWithError(ConsulAgentError, err.Error()) - if err := r.Status().Update(context.Background(), &svcDefaults); err != nil { - return ctrl.Result{}, err - } - return ctrl.Result{}, err - } - svcDefaults.Status.Conditions = syncSuccessful() - if err := r.Status().Update(context.Background(), &svcDefaults); err != nil { - return ctrl.Result{}, err + return r.syncUnknownWithError(logger, svcDefaults, ConsulAgentError, + fmt.Errorf("updating config entry in consul: %w", err)) } + return r.syncSuccessful(svcDefaults) } else if !svcDefaults.Status.GetCondition(consulv1alpha1.ConditionSynced).IsTrue() { - svcDefaults.Status.Conditions = syncSuccessful() - if err := r.Status().Update(context.Background(), &svcDefaults); err != nil { - return ctrl.Result{}, err - } + return r.syncSuccessful(svcDefaults) } return ctrl.Result{}, nil @@ -149,26 +170,50 @@ func (r *ServiceDefaultsReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -func syncFailed(reason, message string) consulv1alpha1.Conditions { - return consulv1alpha1.Conditions{ +// consulNamespace returns the namespace that a service should be +// registered in based on the namespace options. It returns an +// empty string if namespaces aren't enabled. +func (r *ServiceDefaultsReconciler) consulNamespace(ns string) string { + if !r.EnableConsulNamespaces { + return "" + } + + // Mirroring takes precedence. + if r.EnableNSMirroring { + return fmt.Sprintf("%s%s", r.NSMirroringPrefix, ns) + } + + return r.ConsulDestinationNamespace +} + +func (r *ServiceDefaultsReconciler) syncFailed(logger logr.Logger, svcDefaults consulv1alpha1.ServiceDefaults, errType string, err error) (ctrl.Result, error) { + svcDefaults.Status.Conditions = consulv1alpha1.Conditions{ { Type: consulv1alpha1.ConditionSynced, Status: corev1.ConditionFalse, LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, + Reason: errType, + Message: err.Error(), }, } + if updateErr := r.Status().Update(context.Background(), &svcDefaults); updateErr != nil { + // Log the original error here because we are returning the updateErr. + // Otherwise the original error would be lost. + logger.Error(err, "sync failed") + return ctrl.Result{}, updateErr + } + return ctrl.Result{}, err } -func syncSuccessful() consulv1alpha1.Conditions { - return consulv1alpha1.Conditions{ +func (r *ServiceDefaultsReconciler) syncSuccessful(svcDefaults consulv1alpha1.ServiceDefaults) (ctrl.Result, error) { + svcDefaults.Status.Conditions = consulv1alpha1.Conditions{ { Type: consulv1alpha1.ConditionSynced, Status: corev1.ConditionTrue, LastTransitionTime: metav1.Now(), }, } + return ctrl.Result{}, r.Status().Update(context.Background(), &svcDefaults) } func syncUnknown() consulv1alpha1.Conditions { @@ -181,16 +226,23 @@ func syncUnknown() consulv1alpha1.Conditions { } } -func syncUnknownWithError(reason, message string) consulv1alpha1.Conditions { - return consulv1alpha1.Conditions{ +func (r *ServiceDefaultsReconciler) syncUnknownWithError(logger logr.Logger, svcDefaults consulv1alpha1.ServiceDefaults, errType string, err error) (ctrl.Result, error) { + svcDefaults.Status.Conditions = consulv1alpha1.Conditions{ { Type: consulv1alpha1.ConditionSynced, Status: corev1.ConditionUnknown, LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, + Reason: errType, + Message: err.Error(), }, } + if updateErr := r.Status().Update(context.Background(), &svcDefaults); updateErr != nil { + // Log the original error here because we are returning the updateErr. + // Otherwise the original error would be lost. + logger.Error(err, "sync status unknown") + return ctrl.Result{}, updateErr + } + return ctrl.Result{}, err } func isNotFoundErr(err error) bool { diff --git a/controllers/servicedefaults_controller_ent_test.go b/controllers/servicedefaults_controller_ent_test.go new file mode 100644 index 0000000000..705c0ea5ef --- /dev/null +++ b/controllers/servicedefaults_controller_ent_test.go @@ -0,0 +1,390 @@ +// +build enterprise + +package controllers_test + +import ( + "context" + "testing" + "time" + + logrtest "github.com/go-logr/logr/testing" + "github.com/hashicorp/consul-k8s/api/v1alpha1" + "github.com/hashicorp/consul-k8s/controllers" + capi "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/sdk/testutil" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestServiceDefaultsController_createsConfigEntry_consulNamespaces(tt *testing.T) { + cases := map[string]struct { + Mirror bool + MirrorPrefix string + SourceKubeNS string + DestConsulNS string + ExpConsulNS string + }{ + "SourceKubeNS=default, DestConsulNS=default": { + SourceKubeNS: "default", + DestConsulNS: "default", + ExpConsulNS: "default", + }, + "SourceKubeNS=kube, DestConsulNS=default": { + SourceKubeNS: "kube", + DestConsulNS: "default", + ExpConsulNS: "default", + }, + "SourceKubeNS=default, DestConsulNS=other": { + SourceKubeNS: "default", + DestConsulNS: "other", + ExpConsulNS: "other", + }, + "SourceKubeNS=kube, DestConsulNS=other": { + SourceKubeNS: "kube", + DestConsulNS: "other", + ExpConsulNS: "other", + }, + "SourceKubeNS=default, Mirror=true": { + SourceKubeNS: "default", + Mirror: true, + ExpConsulNS: "default", + }, + "SourceKubeNS=kube, Mirror=true": { + SourceKubeNS: "kube", + Mirror: true, + ExpConsulNS: "kube", + }, + "SourceKubeNS=default, Mirror=true, Prefix=prefix": { + SourceKubeNS: "default", + Mirror: true, + MirrorPrefix: "prefix-", + ExpConsulNS: "prefix-default", + }, + } + + for name, c := range cases { + tt.Run(name, func(t *testing.T) { + req := require.New(t) + svcDefaults := &v1alpha1.ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: c.SourceKubeNS, + }, + Spec: v1alpha1.ServiceDefaultsSpec{ + Protocol: "http", + }, + } + s := scheme.Scheme + s.AddKnownTypes(v1alpha1.GroupVersion, svcDefaults) + ctx := context.Background() + + consul, err := testutil.NewTestServerConfigT(t, nil) + req.NoError(err) + defer consul.Stop() + consulClient, err := capi.NewClient(&capi.Config{ + Address: consul.HTTPAddr, + }) + req.NoError(err) + + client := fake.NewFakeClientWithScheme(s, svcDefaults) + + r := controllers.ServiceDefaultsReconciler{ + Client: client, + Log: logrtest.TestLogger{T: t}, + Scheme: s, + ConsulClient: consulClient, + EnableConsulNamespaces: true, + EnableNSMirroring: c.Mirror, + NSMirroringPrefix: c.MirrorPrefix, + ConsulDestinationNamespace: c.DestConsulNS, + } + + resp, err := r.Reconcile(ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: svcDefaults.ObjectMeta.Namespace, + Name: svcDefaults.ObjectMeta.Name, + }, + }) + req.NoError(err) + req.False(resp.Requeue) + + cfg, _, err := consulClient.ConfigEntries().Get(capi.ServiceDefaults, "foo", &capi.QueryOptions{ + Namespace: c.ExpConsulNS, + }) + req.NoError(err) + svcDefault, ok := cfg.(*capi.ServiceConfigEntry) + req.True(ok) + req.Equal("http", svcDefault.Protocol) + + // Check that the status is "synced". + err = client.Get(ctx, types.NamespacedName{ + Namespace: svcDefaults.Namespace, + Name: svcDefaults.Name, + }, svcDefaults) + req.NoError(err) + conditionSynced := svcDefaults.Status.GetCondition(v1alpha1.ConditionSynced) + req.True(conditionSynced.IsTrue()) + + }) + } +} + +func TestServiceDefaultsController_updatesConfigEntry_consulNamespaces(tt *testing.T) { + cases := map[string]struct { + Mirror bool + MirrorPrefix string + SourceKubeNS string + DestConsulNS string + ExpConsulNS string + }{ + "SourceKubeNS=default, DestConsulNS=default": { + SourceKubeNS: "default", + DestConsulNS: "default", + ExpConsulNS: "default", + }, + "SourceKubeNS=kube, DestConsulNS=default": { + SourceKubeNS: "kube", + DestConsulNS: "default", + ExpConsulNS: "default", + }, + "SourceKubeNS=default, DestConsulNS=other": { + SourceKubeNS: "default", + DestConsulNS: "other", + ExpConsulNS: "other", + }, + "SourceKubeNS=kube, DestConsulNS=other": { + SourceKubeNS: "kube", + DestConsulNS: "other", + ExpConsulNS: "other", + }, + "SourceKubeNS=default, Mirror=true": { + SourceKubeNS: "default", + Mirror: true, + ExpConsulNS: "default", + }, + "SourceKubeNS=kube, Mirror=true": { + SourceKubeNS: "kube", + Mirror: true, + ExpConsulNS: "kube", + }, + "SourceKubeNS=default, Mirror=true, Prefix=prefix": { + SourceKubeNS: "default", + Mirror: true, + MirrorPrefix: "prefix-", + ExpConsulNS: "prefix-default", + }, + } + + for name, c := range cases { + tt.Run(name, func(t *testing.T) { + req := require.New(t) + svcDefaults := &v1alpha1.ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: c.SourceKubeNS, + Finalizers: []string{controllers.FinalizerName}, + }, + Spec: v1alpha1.ServiceDefaultsSpec{ + Protocol: "http", + }, + } + s := scheme.Scheme + s.AddKnownTypes(v1alpha1.GroupVersion, svcDefaults) + ctx := context.Background() + + consul, err := testutil.NewTestServerConfigT(t, nil) + req.NoError(err) + defer consul.Stop() + consulClient, err := capi.NewClient(&capi.Config{ + Address: consul.HTTPAddr, + }) + req.NoError(err) + + client := fake.NewFakeClientWithScheme(s, svcDefaults) + + r := controllers.ServiceDefaultsReconciler{ + Client: client, + Log: logrtest.TestLogger{T: t}, + Scheme: s, + ConsulClient: consulClient, + EnableConsulNamespaces: true, + ConsulDestinationNamespace: c.DestConsulNS, + EnableNSMirroring: c.Mirror, + NSMirroringPrefix: c.MirrorPrefix, + } + + // We haven't run reconcile yet so ensure it's created in Consul. + { + if c.ExpConsulNS != "default" { + _, _, err := consulClient.Namespaces().Create(&capi.Namespace{ + Name: c.ExpConsulNS, + }, nil) + req.NoError(err) + } + written, _, err := consulClient.ConfigEntries().Set(&capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "foo", + Protocol: "http", + }, &capi.WriteOptions{Namespace: c.ExpConsulNS}) + req.NoError(err) + req.True(written) + } + + // Now update it. + { + // First get it so we have the latest revision number. + err = client.Get(ctx, types.NamespacedName{ + Namespace: svcDefaults.Namespace, + Name: svcDefaults.Name, + }, svcDefaults) + req.NoError(err) + + // Update the protocol. + svcDefaults.Spec.Protocol = "tcp" + err := client.Update(ctx, svcDefaults) + req.NoError(err) + + resp, err := r.Reconcile(ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: svcDefaults.ObjectMeta.Namespace, + Name: svcDefaults.ObjectMeta.Name, + }, + }) + req.NoError(err) + req.False(resp.Requeue) + + cfg, _, err := consulClient.ConfigEntries().Get(capi.ServiceDefaults, "foo", &capi.QueryOptions{Namespace: c.ExpConsulNS}) + req.NoError(err) + svcDefault, ok := cfg.(*capi.ServiceConfigEntry) + req.True(ok) + req.Equal("tcp", svcDefault.Protocol) + } + }) + } +} + +func TestServiceDefaultsController_deletesConfigEntry_consulNamespaces(tt *testing.T) { + cases := map[string]struct { + Mirror bool + MirrorPrefix string + SourceKubeNS string + DestConsulNS string + ExpConsulNS string + }{ + "SourceKubeNS=default, DestConsulNS=default": { + SourceKubeNS: "default", + DestConsulNS: "default", + ExpConsulNS: "default", + }, + "SourceKubeNS=kube, DestConsulNS=default": { + SourceKubeNS: "kube", + DestConsulNS: "default", + ExpConsulNS: "default", + }, + "SourceKubeNS=default, DestConsulNS=other": { + SourceKubeNS: "default", + DestConsulNS: "other", + ExpConsulNS: "other", + }, + "SourceKubeNS=kube, DestConsulNS=other": { + SourceKubeNS: "kube", + DestConsulNS: "other", + ExpConsulNS: "other", + }, + "SourceKubeNS=default, Mirror=true": { + SourceKubeNS: "default", + Mirror: true, + ExpConsulNS: "default", + }, + "SourceKubeNS=kube, Mirror=true": { + SourceKubeNS: "kube", + Mirror: true, + ExpConsulNS: "kube", + }, + "SourceKubeNS=default, Mirror=true, Prefix=prefix": { + SourceKubeNS: "default", + Mirror: true, + MirrorPrefix: "prefix-", + ExpConsulNS: "prefix-default", + }, + } + + for name, c := range cases { + tt.Run(name, func(t *testing.T) { + req := require.New(t) + // Create it with the deletion timestamp set to mimic that it's already + // been marked for deletion. + svcDefaults := &v1alpha1.ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: c.SourceKubeNS, + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{controllers.FinalizerName}, + }, + Spec: v1alpha1.ServiceDefaultsSpec{ + Protocol: "http", + }, + } + s := scheme.Scheme + s.AddKnownTypes(v1alpha1.GroupVersion, svcDefaults) + + consul, err := testutil.NewTestServerConfigT(t, nil) + req.NoError(err) + defer consul.Stop() + consulClient, err := capi.NewClient(&capi.Config{ + Address: consul.HTTPAddr, + }) + req.NoError(err) + + client := fake.NewFakeClientWithScheme(s, svcDefaults) + + r := controllers.ServiceDefaultsReconciler{ + Client: client, + Log: logrtest.TestLogger{T: t}, + Scheme: s, + ConsulClient: consulClient, + EnableConsulNamespaces: true, + ConsulDestinationNamespace: c.DestConsulNS, + EnableNSMirroring: c.Mirror, + NSMirroringPrefix: c.MirrorPrefix, + } + + // We haven't run reconcile yet so ensure it's created in Consul. + { + if c.ExpConsulNS != "default" { + _, _, err := consulClient.Namespaces().Create(&capi.Namespace{ + Name: c.ExpConsulNS, + }, nil) + req.NoError(err) + } + + written, _, err := consulClient.ConfigEntries().Set(&capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "foo", + Protocol: "http", + }, &capi.WriteOptions{Namespace: c.ExpConsulNS}) + req.NoError(err) + req.True(written) + } + + // Now run reconcile. It's marked for deletion so this should delete it. + { + resp, err := r.Reconcile(ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: svcDefaults.ObjectMeta.Namespace, + Name: svcDefaults.ObjectMeta.Name, + }, + }) + req.NoError(err) + req.False(resp.Requeue) + + _, _, err = consulClient.ConfigEntries().Get(capi.ServiceDefaults, "foo", &capi.QueryOptions{Namespace: c.ExpConsulNS}) + req.EqualError(err, "Unexpected response code: 404 (Config entry not found for \"service-defaults\" / \"foo\")") + } + }) + } +} diff --git a/controllers/servicedefaults_controller_test.go b/controllers/servicedefaults_controller_test.go index 99afcab346..73afb33882 100644 --- a/controllers/servicedefaults_controller_test.go +++ b/controllers/servicedefaults_controller_test.go @@ -45,7 +45,7 @@ func TestServiceDefaultsController_createsConfigEntry(t *testing.T) { r := controllers.ServiceDefaultsReconciler{ Client: client, - Log: logrtest.NullLogger{}, + Log: logrtest.TestLogger{T: t}, Scheme: s, ConsulClient: consulClient, } @@ -101,7 +101,7 @@ func TestServiceDefaultsController_addsFinalizerOnCreate(t *testing.T) { r := controllers.ServiceDefaultsReconciler{ Client: client, - Log: logrtest.NullLogger{}, + Log: logrtest.TestLogger{T: t}, Scheme: s, ConsulClient: consulClient, } @@ -153,7 +153,7 @@ func TestServiceDefaultsController_updatesConfigEntry(t *testing.T) { r := controllers.ServiceDefaultsReconciler{ Client: client, - Log: logrtest.NullLogger{}, + Log: logrtest.TestLogger{T: t}, Scheme: s, ConsulClient: consulClient, } @@ -230,7 +230,7 @@ func TestServiceDefaultsController_deletesConfigEntry(t *testing.T) { r := controllers.ServiceDefaultsReconciler{ Client: client, - Log: logrtest.NullLogger{}, + Log: logrtest.TestLogger{T: t}, Scheme: s, ConsulClient: consulClient, } @@ -286,7 +286,7 @@ func TestServiceDefaultsController_errorUpdatesSyncStatus(t *testing.T) { r := controllers.ServiceDefaultsReconciler{ Client: client, - Log: logrtest.NullLogger{}, + Log: logrtest.TestLogger{T: t}, Scheme: s, ConsulClient: consulClient, } @@ -297,7 +297,8 @@ func TestServiceDefaultsController_errorUpdatesSyncStatus(t *testing.T) { Name: svcDefaults.Name, }, }) - req.EqualError(err, "Get \"http://incorrect-address/v1/config/service-defaults/foo\": dial tcp: lookup incorrect-address on 127.0.0.11:53: no such host") + req.Error(err) + req.Contains(err.Error(), "Get \"http://incorrect-address/v1/config/service-defaults/foo\": dial tcp: lookup incorrect-address") req.False(resp.Requeue) // Check that the status is "synced=false". @@ -309,5 +310,5 @@ func TestServiceDefaultsController_errorUpdatesSyncStatus(t *testing.T) { conditionSynced := svcDefaults.Status.GetCondition(v1alpha1.ConditionSynced) req.True(conditionSynced.IsFalse()) req.Equal("ConsulAgentError", conditionSynced.Reason) - req.Equal("Get \"http://incorrect-address/v1/config/service-defaults/foo\": dial tcp: lookup incorrect-address on 127.0.0.11:53: no such host", conditionSynced.Message) + req.Contains(conditionSynced.Message, "Get \"http://incorrect-address/v1/config/service-defaults/foo\": dial tcp: lookup incorrect-address", conditionSynced.Message) } diff --git a/namespaces/namespaces.go b/namespaces/namespaces.go new file mode 100644 index 0000000000..b6348d8671 --- /dev/null +++ b/namespaces/namespaces.go @@ -0,0 +1,40 @@ +// Package namespaces handles interaction with Consul namespaces needed across +// commands. +package namespaces + +import capi "github.com/hashicorp/consul/api" + +// EnsureExists ensures a Consul namespace with name ns exists. If it doesn't, +// it will create it and set crossNSACLPolicy as a policy default. +func EnsureExists(client *capi.Client, ns string, crossNSAClPolicy string) error { + // Check if the Consul namespace exists. + namespaceInfo, _, err := client.Namespaces().Read(ns, nil) + if err != nil { + return err + } + if namespaceInfo != nil { + return nil + } + + // If not, create it. + var aclConfig capi.NamespaceACLConfig + if crossNSAClPolicy != "" { + // Create the ACLs config for the cross-Consul-namespace + // default policy that needs to be attached + aclConfig = capi.NamespaceACLConfig{ + PolicyDefaults: []capi.ACLLink{ + {Name: crossNSAClPolicy}, + }, + } + } + + consulNamespace := capi.Namespace{ + Name: ns, + Description: "Auto-generated by consul-k8s", + ACLs: &aclConfig, + Meta: map[string]string{"external-source": "kubernetes"}, + } + + _, _, err = client.Namespaces().Create(&consulNamespace, nil) + return err +} diff --git a/namespaces/namespaces_test.go b/namespaces/namespaces_test.go new file mode 100644 index 0000000000..012ad6373e --- /dev/null +++ b/namespaces/namespaces_test.go @@ -0,0 +1,91 @@ +// +build enterprise + +package namespaces + +import ( + "testing" + "time" + + capi "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/stretchr/testify/require" +) + +// Test that if the namespace already exists the function succeeds. +func TestEnsureExists_AlreadyExists(t *testing.T) { + req := require.New(t) + ns := "ns" + + consul, err := testutil.NewTestServerConfigT(t, nil) + req.NoError(err) + defer consul.Stop() + consulClient, err := capi.NewClient(&capi.Config{ + Address: consul.HTTPAddr, + }) + req.NoError(err) + + // Pre-create the namespace. + _, _, err = consulClient.Namespaces().Create(&capi.Namespace{ + Name: ns, + }, nil) + req.NoError(err) + + err = EnsureExists(consulClient, ns, "cross-ns-policy") + req.NoError(err) + + // Ensure it still exists. + _, _, err = consulClient.Namespaces().Read(ns, nil) + req.NoError(err) +} + +// Test that it creates the namespace if it doesn't exist. +func TestEnsureExists_CreatesNS(t *testing.T) { + req := require.New(t) + ns := "ns" + crossNSPolicy := "cross-ns-policy" + + consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + c.ACL.Enabled = true + }) + req.NoError(err) + defer consul.Stop() + + // Set up a client for bootstrapping + bootClient, err := capi.NewClient(&capi.Config{ + Address: consul.HTTPAddr, + }) + require.NoError(t, err) + + // Bootstrap the server and get the bootstrap token + var bootstrapResp *capi.ACLToken + timer := &retry.Timer{Timeout: 10 * time.Second, Wait: 500 * time.Millisecond} + retry.RunWith(timer, t, func(r *retry.R) { + bootstrapResp, _, err = bootClient.ACL().Bootstrap() + require.NoError(r, err) + }) + bootstrapToken := bootstrapResp.SecretID + require.NotEmpty(t, bootstrapToken) + + consulClient, err := capi.NewClient(&capi.Config{ + Address: consul.HTTPAddr, + Token: bootstrapToken, + }) + req.NoError(err) + + // Must pre-create the cross-ns policy. + _, _, err = consulClient.ACL().PolicyCreate(&capi.ACLPolicy{ + Name: crossNSPolicy, + }, nil) + req.NoError(err) + + err = EnsureExists(consulClient, ns, crossNSPolicy) + req.NoError(err) + + // Ensure it was created. + cNS, _, err := consulClient.Namespaces().Read(ns, nil) + req.NoError(err) + req.Equal("Auto-generated by consul-k8s", cNS.Description) + req.Len(cNS.ACLs.PolicyDefaults, 1) + req.Equal(cNS.ACLs.PolicyDefaults[0].Name, crossNSPolicy) +} diff --git a/subcommand/controller/command.go b/subcommand/controller/command.go index 0c8cda8c00..5ebfd4a5eb 100644 --- a/subcommand/controller/command.go +++ b/subcommand/controller/command.go @@ -25,6 +25,13 @@ type Command struct { flagMetricsAddr string flagEnableLeaderElection bool + // Flags to support Consul Enterprise namespaces. + flagEnableNamespaces bool + flagConsulDestinationNamespace string + flagEnableNSMirroring bool + flagNSMirroringPrefix string + flagCrossNSACLPolicy string + once sync.Once help string } @@ -46,14 +53,27 @@ func (c *Command) init() { c.flagSet.BoolVar(&c.flagEnableLeaderElection, "enable-leader-election", false, "Enable leader election for controller. "+ "Enabling this will ensure there is only one active controller manager.") + c.flagSet.BoolVar(&c.flagEnableNamespaces, "enable-namespaces", false, + "[Enterprise Only] Enables Consul Enterprise namespaces, in either a single Consul namespace or mirrored") + c.flagSet.StringVar(&c.flagConsulDestinationNamespace, "consul-destination-namespace", "default", + "[Enterprise Only] Defines which Consul namespace to create all config entries in, regardless of their source Kubernetes namespace."+ + " If '-enable-k8s-namespace-mirroring' is true, this is not used.") + c.flagSet.BoolVar(&c.flagEnableNSMirroring, "enable-k8s-namespace-mirroring", false, "[Enterprise Only] Enables "+ + "k8s namespace mirroring.") + c.flagSet.StringVar(&c.flagNSMirroringPrefix, "k8s-namespace-mirroring-prefix", "", + "[Enterprise Only] Prefix that will be added to all k8s namespaces mirrored into Consul if mirroring is enabled.") + c.flagSet.StringVar(&c.flagCrossNSACLPolicy, "consul-cross-namespace-acl-policy", "", + "[Enterprise Only] Name of the ACL policy to attach to all created Consul namespaces to allow service "+ + "discovery across Consul namespaces. Only necessary if ACLs are enabled.") + c.httpFlags = &flags.HTTPFlags{} flags.Merge(c.flagSet, c.httpFlags.Flags()) c.help = flags.Usage(help, c.flagSet) } -func (c *Command) Run(_ []string) int { +func (c *Command) Run(args []string) int { c.once.Do(c.init) - if err := c.flagSet.Parse(nil); err != nil { + if err := c.flagSet.Parse(args); err != nil { setupLog.Error(err, "parsing flagSet") return 1 } @@ -83,10 +103,15 @@ func (c *Command) Run(_ []string) int { } if err = (&controllers.ServiceDefaultsReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("ServiceDefaults"), - Scheme: mgr.GetScheme(), - ConsulClient: consulClient, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("ServiceDefaults"), + Scheme: mgr.GetScheme(), + ConsulClient: consulClient, + EnableConsulNamespaces: c.flagEnableNamespaces, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + EnableNSMirroring: c.flagEnableNSMirroring, + NSMirroringPrefix: c.flagNSMirroringPrefix, + CrossNSACLPolicy: c.flagCrossNSACLPolicy, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ServiceDefaults") return 1 diff --git a/subcommand/delete-completed-job/command_test.go b/subcommand/delete-completed-job/command_test.go index a07054e06b..0da056fc88 100644 --- a/subcommand/delete-completed-job/command_test.go +++ b/subcommand/delete-completed-job/command_test.go @@ -34,7 +34,7 @@ func TestRun_ArgValidation(t *testing.T) { }, { []string{"-k8s-namespace=default", "-timeout=10jd", "job-name"}, - "\"10jd\" is not a valid timeout: time: unknown unit jd in duration 10jd", + "\"10jd\" is not a valid timeout", }, } for _, c := range cases { diff --git a/subcommand/flags/mapset.go b/subcommand/flags/mapset.go new file mode 100644 index 0000000000..c58cc9a3a2 --- /dev/null +++ b/subcommand/flags/mapset.go @@ -0,0 +1,12 @@ +package flags + +import "github.com/deckarep/golang-set" + +// ToSet creates a set from s. +func ToSet(s []string) mapset.Set { + set := mapset.NewSet() + for _, allow := range s { + set.Add(allow) + } + return set +} diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go index 97c56cf8f1..05157111e0 100644 --- a/subcommand/inject-connect/command.go +++ b/subcommand/inject-connect/command.go @@ -13,7 +13,6 @@ import ( "sync/atomic" "time" - "github.com/deckarep/golang-set" "github.com/hashicorp/consul-k8s/connect-inject" "github.com/hashicorp/consul-k8s/helper/cert" "github.com/hashicorp/consul-k8s/subcommand/flags" @@ -114,12 +113,12 @@ func (c *Command) init() { c.flagSet.Var((*flags.AppendSliceValue)(&c.flagDenyK8sNamespacesList), "deny-k8s-namespace", "K8s namespaces to explicitly deny. Takes precedence over allow. May be specified multiple times.") c.flagSet.BoolVar(&c.flagEnableNamespaces, "enable-namespaces", false, - "[Enterprise Only] Enables namespaces, in either a single Consul namespace or mirrored") + "[Enterprise Only] Enables namespaces, in either a single Consul namespace or mirrored.") c.flagSet.StringVar(&c.flagConsulDestinationNamespace, "consul-destination-namespace", "default", - "[Enterprise Only] Defines which Consul namespace to register all injected services into. If '-enable-namespace-mirroring' "+ + "[Enterprise Only] Defines which Consul namespace to register all injected services into. If '-enable-k8s-namespace-mirroring' "+ "is true, this is not used.") c.flagSet.BoolVar(&c.flagEnableK8SNSMirroring, "enable-k8s-namespace-mirroring", false, "[Enterprise Only] Enables "+ - "k8s namespace mirroring") + "k8s namespace mirroring.") c.flagSet.StringVar(&c.flagK8SNSMirroringPrefix, "k8s-namespace-mirroring-prefix", "", "[Enterprise Only] Prefix that will be added to all k8s namespaces mirrored into Consul if mirroring is enabled.") c.flagSet.StringVar(&c.flagCrossNamespaceACLPolicy, "consul-cross-namespace-acl-policy", "", @@ -278,14 +277,8 @@ func (c *Command) Run(args []string) int { go c.certWatcher(ctx, certCh, c.clientset) // Convert allow/deny lists to sets - allowSet := mapset.NewSet() - denySet := mapset.NewSet() - for _, allow := range c.flagAllowK8sNamespacesList { - allowSet.Add(allow) - } - for _, deny := range c.flagDenyK8sNamespacesList { - denySet.Add(deny) - } + allowK8sNamespaces := flags.ToSet(c.flagAllowK8sNamespacesList) + denyK8sNamespaces := flags.ToSet(c.flagDenyK8sNamespacesList) // Build the HTTP handler and server injector := connectinject.Handler{ @@ -305,8 +298,8 @@ func (c *Command) Run(args []string) int { InitContainerResources: initResources, LifecycleSidecarResources: lifecycleResources, EnableNamespaces: c.flagEnableNamespaces, - AllowK8sNamespacesSet: allowSet, - DenyK8sNamespacesSet: denySet, + AllowK8sNamespacesSet: allowK8sNamespaces, + DenyK8sNamespacesSet: denyK8sNamespaces, ConsulDestinationNamespace: c.flagConsulDestinationNamespace, EnableK8SNSMirroring: c.flagEnableK8SNSMirroring, K8SNSMirroringPrefix: c.flagK8SNSMirroringPrefix, diff --git a/subcommand/server-acl-init/command_ent_test.go b/subcommand/server-acl-init/command_ent_test.go index 65f3aebf1c..000648539e 100644 --- a/subcommand/server-acl-init/command_ent_test.go +++ b/subcommand/server-acl-init/command_ent_test.go @@ -105,7 +105,7 @@ func TestRun_ConnectInject_SingleDestinationNamespace(t *testing.T) { require.NoError(err) require.NotNil(actNamespace) require.Equal(consulDestNamespace, actNamespace.Name) - require.Equal("Auto-generated by the ACL bootstrapping process", actNamespace.Description) + require.Equal("Auto-generated by consul-k8s", actNamespace.Description) require.NotNil(actNamespace.ACLs) require.Len(actNamespace.ACLs.PolicyDefaults, 1) require.Equal("cross-namespace-policy", actNamespace.ACLs.PolicyDefaults[0].Name) diff --git a/subcommand/server-acl-init/connect_inject.go b/subcommand/server-acl-init/connect_inject.go index 586e3ae3e7..d8db88ad30 100644 --- a/subcommand/server-acl-init/connect_inject.go +++ b/subcommand/server-acl-init/connect_inject.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/hashicorp/consul-k8s/namespaces" "github.com/hashicorp/consul/api" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,7 +47,7 @@ func (c *Command) configureConnectInject(consulClient *api.Client) error { err = c.untilSucceeds(fmt.Sprintf("checking or creating namespace %s", c.flagConsulInjectDestinationNamespace), func() error { - err := c.checkAndCreateNamespace(c.flagConsulInjectDestinationNamespace, consulClient) + err := namespaces.EnsureExists(consulClient, c.flagConsulInjectDestinationNamespace, "cross-namespace-policy") return err }) if err != nil { diff --git a/subcommand/server-acl-init/create_or_update.go b/subcommand/server-acl-init/create_or_update.go index c8b4b28f58..a696b01708 100644 --- a/subcommand/server-acl-init/create_or_update.go +++ b/subcommand/server-acl-init/create_or_update.go @@ -143,40 +143,6 @@ func (c *Command) createOrUpdateACLPolicy(policy api.ACLPolicy, consulClient *ap return err } -func (c *Command) checkAndCreateNamespace(ns string, consulClient *api.Client) error { - // Check if the Consul namespace exists - namespaceInfo, _, err := consulClient.Namespaces().Read(ns, nil) - if err != nil { - return err - } - - // If not, create it - if namespaceInfo == nil { - // Create the ACLs config for the cross-Consul-namespace - // default policy that needs to be attached - aclConfig := api.NamespaceACLConfig{ - PolicyDefaults: []api.ACLLink{ - {Name: "cross-namespace-policy"}, - }, - } - - consulNamespace := api.Namespace{ - Name: ns, - Description: "Auto-generated by the ACL bootstrapping process", - ACLs: &aclConfig, - Meta: map[string]string{"external-source": "kubernetes"}, - } - - _, _, err = consulClient.Namespaces().Create(&consulNamespace, nil) - if err != nil { - return err - } - c.log.Info("created consul namespace", "name", consulNamespace.Name) - } - - return nil -} - // isPolicyExistsErr returns true if err is due to trying to call the // policy create API when the policy already exists. func isPolicyExistsErr(err error, policyName string) bool { diff --git a/subcommand/sync-catalog/command.go b/subcommand/sync-catalog/command.go index cc49e2727b..4c0fd45593 100644 --- a/subcommand/sync-catalog/command.go +++ b/subcommand/sync-catalog/command.go @@ -120,12 +120,12 @@ func (c *Command) init() { c.flags.Var((*flags.AppendSliceValue)(&c.flagDenyK8sNamespacesList), "deny-k8s-namespace", "K8s namespaces to explicitly deny. Takes precedence over allow. May be specified multiple times.") c.flags.BoolVar(&c.flagEnableNamespaces, "enable-namespaces", false, - "[Enterprise Only] Enables namespaces, in either a single Consul namespace or mirrored") + "[Enterprise Only] Enables namespaces, in either a single Consul namespace or mirrored.") c.flags.StringVar(&c.flagConsulDestinationNamespace, "consul-destination-namespace", "default", - "[Enterprise Only] Defines which Consul namespace to register all synced services into. If 'enable-namespace-mirroring' "+ + "[Enterprise Only] Defines which Consul namespace to register all synced services into. If '-enable-k8s-namespace-mirroring' "+ "is true, this is not used.") c.flags.BoolVar(&c.flagEnableK8SNSMirroring, "enable-k8s-namespace-mirroring", false, "[Enterprise Only] Enables "+ - "namespace mirroring") + "namespace mirroring.") c.flags.StringVar(&c.flagK8SNSMirroringPrefix, "k8s-namespace-mirroring-prefix", "", "[Enterprise Only] Prefix that will be added to all k8s namespaces mirrored into Consul if mirroring is enabled.") c.flags.StringVar(&c.flagCrossNamespaceACLPolicy, "consul-cross-namespace-acl-policy", "", @@ -197,19 +197,12 @@ func (c *Command) Run(args []string) int { } // Convert allow/deny lists to sets - allowSet := mapset.NewSet() - denySet := mapset.NewSet() + allowSet := flags.ToSet(c.flagAllowK8sNamespacesList) + denySet := flags.ToSet(c.flagDenyK8sNamespacesList) if c.flagK8SSourceNamespace != "" { // For backwards compatibility, if `flagK8SSourceNamespace` is set, // it will be the only allowed namespace - allowSet.Add(c.flagK8SSourceNamespace) - } else { - for _, allow := range c.flagAllowK8sNamespacesList { - allowSet.Add(allow) - } - for _, deny := range c.flagDenyK8sNamespacesList { - denySet.Add(deny) - } + allowSet = mapset.NewSet(c.flagK8SSourceNamespace) } c.logger.Info("K8s namespace syncing configuration", "k8s namespaces allowed to be synced", allowSet, "k8s namespaces denied from syncing", denySet) diff --git a/subcommand/sync-catalog/command_ent_test.go b/subcommand/sync-catalog/command_ent_test.go index 694538b742..80af158ea4 100644 --- a/subcommand/sync-catalog/command_ent_test.go +++ b/subcommand/sync-catalog/command_ent_test.go @@ -116,7 +116,7 @@ func TestRun_ToConsulSingleDestinationNamespace(t *testing.T) { // Check created namespace properties if ns != "default" { - require.Equalf(r, "Auto-generated by a Catalog Sync Process", actNamespace.Description, + require.Equalf(r, "Auto-generated by consul-k8s", actNamespace.Description, "wrong namespace description for namespace %s", ns) require.Containsf(r, actNamespace.Meta, "external-source", "namespace %s does not contain external-source metadata key", ns) @@ -266,7 +266,7 @@ func TestRun_ToConsulMirroringNamespaces(t *testing.T) { // Check created namespace properties if ns != "default" { - require.Equalf(r, "Auto-generated by a Catalog Sync Process", actNamespace.Description, + require.Equalf(r, "Auto-generated by consul-k8s", actNamespace.Description, "wrong namespace description for namespace %s", ns) require.Containsf(r, actNamespace.Meta, "external-source", "namespace %s does not contain external-source metadata key", ns) @@ -712,7 +712,7 @@ func TestRun_ToConsulNamespacesACLs(t *testing.T) { // Check created namespace properties if ns != "default" { - require.Equalf(r, "Auto-generated by a Catalog Sync Process", actNamespace.Description, + require.Equalf(r, "Auto-generated by consul-k8s", actNamespace.Description, "wrong namespace description for namespace %s", ns) require.Containsf(r, actNamespace.Meta, "external-source", "namespace %s does not contain external-source metadata key", ns)