diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d93944286..66f20fb3c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ BUG FIXES: * CRDs: Fix issue where a `ServiceIntentions` resource could be continually resynced with Consul because Consul's internal representation had a different order for an array than the Kubernetes resource. [[GH-416](https://github.com/hashicorp/consul-k8s/pull/416)] +* CRDs: **(Consul Enterprise only)** default the `namespace` fields on resources where Consul performs namespace defaulting to prevent constant re-syncing. + [[GH-413](https://github.com/hashicorp/consul-k8s/pull/413)] ## 0.22.0 (December 21, 2020) diff --git a/api/common/configentry.go b/api/common/configentry.go index 5aec9ebee0..063361aecb 100644 --- a/api/common/configentry.go +++ b/api/common/configentry.go @@ -57,4 +57,7 @@ type ConfigEntryResource interface { DeepCopyObject() runtime.Object // Validate returns an error if the resource is invalid. Validate(namespacesEnabled bool) error + // DefaultNamespaceFields sets Consul namespace fields on the config entry + // spec to their default values if namespaces are enabled. + DefaultNamespaceFields(consulNamespacesEnabled bool, destinationNamespace string, mirroring bool, prefix string) } diff --git a/api/common/configentry_webhook.go b/api/common/configentry_webhook.go index 10a9e46044..034665b66a 100644 --- a/api/common/configentry_webhook.go +++ b/api/common/configentry_webhook.go @@ -2,10 +2,12 @@ package common import ( "context" + "encoding/json" "fmt" "net/http" "github.com/go-logr/logr" + "gomodules.xyz/jsonpatch/v2" "k8s.io/api/admission/v1beta1" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) @@ -28,8 +30,14 @@ func ValidateConfigEntry( configEntryLister ConfigEntryLister, cfgEntry ConfigEntryResource, enableConsulNamespaces bool, - nsMirroring bool) admission.Response { + nsMirroring bool, + consulDestinationNamespace string, + nsMirroringPrefix string) admission.Response { + defaultingPatches, err := DefaultingPatches(cfgEntry, enableConsulNamespaces, nsMirroring, consulDestinationNamespace, nsMirroringPrefix) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } // On create we need to validate that there isn't already a resource with // the same name in a different namespace if we're need to mapping all Kube // resources to a single Consul namespace. The only case where we're not @@ -56,5 +64,25 @@ func ValidateConfigEntry( if err := cfgEntry.Validate(enableConsulNamespaces); err != nil { return admission.Errored(http.StatusBadRequest, err) } - return admission.Allowed(fmt.Sprintf("valid %s request", cfgEntry.KubeKind())) + return admission.Patched(fmt.Sprintf("valid %s request", cfgEntry.KubeKind()), defaultingPatches...) +} + +// DefaultingPatches returns the patches needed to set fields to their +// defaults. +func DefaultingPatches(cfgEntry ConfigEntryResource, enableConsulNamespaces bool, nsMirroring bool, consulDestinationNamespace string, nsMirroringPrefix string) ([]jsonpatch.Operation, error) { + beforeDefaulting, err := json.Marshal(cfgEntry) + if err != nil { + return nil, fmt.Errorf("marshalling input: %s", err) + } + cfgEntry.DefaultNamespaceFields(enableConsulNamespaces, consulDestinationNamespace, nsMirroring, nsMirroringPrefix) + afterDefaulting, err := json.Marshal(cfgEntry) + if err != nil { + return nil, fmt.Errorf("marshalling after defaulting: %s", err) + } + + defaultingPatches, err := jsonpatch.CreatePatch(beforeDefaulting, afterDefaulting) + if err != nil { + return nil, fmt.Errorf("creating patches: %s", err) + } + return defaultingPatches, nil } diff --git a/api/common/configentry_webhook_test.go b/api/common/configentry_webhook_test.go index a8d7d754fe..f49b3ea453 100644 --- a/api/common/configentry_webhook_test.go +++ b/api/common/configentry_webhook_test.go @@ -9,6 +9,7 @@ import ( logrtest "github.com/go-logr/logr/testing" capi "github.com/hashicorp/consul/api" "github.com/stretchr/testify/require" + "gomodules.xyz/jsonpatch/v2" "k8s.io/api/admission/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -21,12 +22,14 @@ func TestValidateConfigEntry(t *testing.T) { otherNS := "other" cases := map[string]struct { - existingResources []ConfigEntryResource - newResource ConfigEntryResource - enableNamespaces bool - nsMirroring bool - expAllow bool - expErrMessage string + existingResources []ConfigEntryResource + newResource ConfigEntryResource + enableNamespaces bool + nsMirroring bool + consulDestinationNS string + nsMirroringPrefix string + expAllow bool + expErrMessage string }{ "no duplicates, valid": { existingResources: nil, @@ -112,7 +115,9 @@ func TestValidateConfigEntry(t *testing.T) { lister, c.newResource, c.enableNamespaces, - c.nsMirroring) + c.nsMirroring, + c.consulDestinationNS, + c.nsMirroringPrefix) require.Equal(t, c.expAllow, response.Allowed) if c.expErrMessage != "" { require.Equal(t, c.expErrMessage, response.AdmissionResponse.Result.Message) @@ -121,6 +126,25 @@ func TestValidateConfigEntry(t *testing.T) { } } +func TestDefaultingPatches(t *testing.T) { + cfgEntry := &mockConfigEntry{ + MockName: "test", + Valid: true, + } + + // This test validates that DefaultingPatches invokes DefaultNamespaceFields on the Config Entry. + patches, err := DefaultingPatches(cfgEntry, false, false, "", "") + require.NoError(t, err) + + require.Equal(t, []jsonpatch.Operation{ + { + Operation: "replace", + Path: "/MockNamespace", + Value: "bar", + }, + }, patches) +} + type mockConfigEntryLister struct { Resources []ConfigEntryResource } @@ -200,6 +224,10 @@ func (in *mockConfigEntry) Validate(bool) error { return nil } +func (in *mockConfigEntry) DefaultNamespaceFields(consulNamespacesEnabled bool, destinationNamespace string, mirroring bool, prefix string) { + in.MockNamespace = "bar" +} + func (in *mockConfigEntry) MatchesConsul(_ capi.ConfigEntry) bool { return false } diff --git a/api/v1alpha1/ingressgateway_types.go b/api/v1alpha1/ingressgateway_types.go index 39c4110a05..40d3d9c1d3 100644 --- a/api/v1alpha1/ingressgateway_types.go +++ b/api/v1alpha1/ingressgateway_types.go @@ -6,6 +6,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/consul-k8s/namespaces" capi "github.com/hashicorp/consul/api" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -225,6 +226,25 @@ func (in *IngressGateway) Validate(namespacesEnabled bool) error { return nil } +// DefaultNamespaceFields sets the namespace field on spec.listeners[].services to their default values if namespaces are enabled. +func (in *IngressGateway) DefaultNamespaceFields(consulNamespacesEnabled bool, destinationNamespace string, mirroring bool, prefix string) { + // If namespaces are enabled we want to set the namespace fields to their + // defaults. If namespaces are not enabled (i.e. OSS) we don't set the + // namespace fields because this would cause errors + // making API calls (because namespace fields can't be set in OSS). + if consulNamespacesEnabled { + // Default to the current namespace (i.e. the namespace of the config entry). + namespace := namespaces.ConsulNamespace(in.Namespace, consulNamespacesEnabled, destinationNamespace, mirroring, prefix) + for i, listener := range in.Spec.Listeners { + for j, service := range listener.Services { + if service.Namespace == "" { + in.Spec.Listeners[i].Services[j].Namespace = namespace + } + } + } + } +} + func (in GatewayTLSConfig) toConsul() capi.GatewayTLSConfig { return capi.GatewayTLSConfig{ Enabled: in.Enabled, diff --git a/api/v1alpha1/ingressgateway_types_test.go b/api/v1alpha1/ingressgateway_types_test.go index 1a5c0247ee..789320ef6e 100644 --- a/api/v1alpha1/ingressgateway_types_test.go +++ b/api/v1alpha1/ingressgateway_types_test.go @@ -3,6 +3,7 @@ package v1alpha1 import ( "testing" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/consul-k8s/api/common" capi "github.com/hashicorp/consul/api" "github.com/stretchr/testify/require" @@ -459,6 +460,98 @@ func TestIngressGateway_Validate(t *testing.T) { } } +// Test defaulting behavior when namespaces are enabled as well as disabled. +func TestIngressGateway_DefaultNamespaceFields(t *testing.T) { + namespaceConfig := map[string]struct { + enabled bool + destinationNamespace string + mirroring bool + prefix string + expectedDestination string + }{ + "disabled": { + enabled: false, + destinationNamespace: "", + mirroring: false, + prefix: "", + expectedDestination: "", + }, + "destinationNS": { + enabled: true, + destinationNamespace: "foo", + mirroring: false, + prefix: "", + expectedDestination: "foo", + }, + "mirroringEnabledWithoutPrefix": { + enabled: true, + destinationNamespace: "", + mirroring: true, + prefix: "", + expectedDestination: "bar", + }, + "mirroringWithPrefix": { + enabled: true, + destinationNamespace: "", + mirroring: true, + prefix: "ns-", + expectedDestination: "ns-bar", + }, + } + + for name, s := range namespaceConfig { + t.Run(name, func(t *testing.T) { + input := &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: IngressGatewaySpec{ + Listeners: []IngressListener{ + { + Protocol: "tcp", + Services: []IngressService{ + { + Name: "name", + }, + { + Name: "other-name", + Namespace: "other", + }, + }, + }, + }, + }, + } + output := &IngressGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: IngressGatewaySpec{ + Listeners: []IngressListener{ + { + Protocol: "tcp", + Services: []IngressService{ + { + Name: "name", + Namespace: s.expectedDestination, + }, + { + Name: "other-name", + Namespace: "other", + }, + }, + }, + }, + }, + } + input.DefaultNamespaceFields(s.enabled, s.destinationNamespace, s.mirroring, s.prefix) + require.True(t, cmp.Equal(input, output)) + }) + } +} + func TestIngressGateway_AddFinalizer(t *testing.T) { resource := &IngressGateway{} resource.AddFinalizer("finalizer") diff --git a/api/v1alpha1/ingressgateway_webhook.go b/api/v1alpha1/ingressgateway_webhook.go index 447de8a294..b78852c230 100644 --- a/api/v1alpha1/ingressgateway_webhook.go +++ b/api/v1alpha1/ingressgateway_webhook.go @@ -26,6 +26,18 @@ type IngressGatewayWebhook struct { // be created in the matching Consul namespace. EnableNSMirroring bool + // ConsulDestinationNamespace is the namespace in Consul that the config entry created + // in k8s will get mapped into. If the Consul namespace does not already exist, it will + // be created. + ConsulDestinationNamespace string + + // NSMirroringPrefix works in conjunction with Namespace Mirroring. + // It is the prefix added to the Consul namespace to map to a specific. + // k8s namespace. For example, if `mirroringK8SPrefix` is set to "k8s-", a + // service in the k8s `staging` namespace will be registered into the + // `k8s-staging` Consul namespace. + NSMirroringPrefix string + decoder *admission.Decoder client.Client } @@ -51,7 +63,9 @@ func (v *IngressGatewayWebhook) Handle(ctx context.Context, req admission.Reques v, &resource, v.EnableConsulNamespaces, - v.EnableNSMirroring) + v.EnableNSMirroring, + v.ConsulDestinationNamespace, + v.NSMirroringPrefix) } func (v *IngressGatewayWebhook) List(ctx context.Context) ([]common.ConfigEntryResource, error) { diff --git a/api/v1alpha1/proxydefaults_types.go b/api/v1alpha1/proxydefaults_types.go index 02445ddcf2..07c6470a61 100644 --- a/api/v1alpha1/proxydefaults_types.go +++ b/api/v1alpha1/proxydefaults_types.go @@ -176,6 +176,11 @@ func (in *ProxyDefaults) Validate(namespacesEnabled bool) error { return nil } +// DefaultNamespaceFields has no behaviour here as proxy-defaults have no namespace specific fields. +func (in *ProxyDefaults) DefaultNamespaceFields(_ bool, _ string, _ bool, _ string) { + return +} + // convertConfig converts the config of type json.RawMessage which is stored // by the resource into type map[string]interface{} which is saved by the // consul API. diff --git a/api/v1alpha1/servicedefaults_types.go b/api/v1alpha1/servicedefaults_types.go index e75f2cfb21..f60445adce 100644 --- a/api/v1alpha1/servicedefaults_types.go +++ b/api/v1alpha1/servicedefaults_types.go @@ -185,6 +185,11 @@ func (in *ServiceDefaults) Validate(namespacesEnabled bool) error { return nil } +// DefaultNamespaceFields has no behaviour here as service-defaults have no namespace specific fields. +func (in *ServiceDefaults) DefaultNamespaceFields(_ bool, _ string, _ bool, _ string) { + return +} + // MatchesConsul returns true if entry has the same config as this struct. func (in *ServiceDefaults) MatchesConsul(candidate capi.ConfigEntry) bool { configEntry, ok := candidate.(*capi.ServiceConfigEntry) diff --git a/api/v1alpha1/servicedefaults_webhook.go b/api/v1alpha1/servicedefaults_webhook.go index e45e8e5ad2..5b5f600eba 100644 --- a/api/v1alpha1/servicedefaults_webhook.go +++ b/api/v1alpha1/servicedefaults_webhook.go @@ -26,6 +26,18 @@ type ServiceDefaultsWebhook struct { // be created in the matching Consul namespace. EnableNSMirroring bool + // ConsulDestinationNamespace is the namespace in Consul that the config entry created + // in k8s will get mapped into. If the Consul namespace does not already exist, it will + // be created. + ConsulDestinationNamespace string + + // NSMirroringPrefix works in conjunction with Namespace Mirroring. + // It is the prefix added to the Consul namespace to map to a specific. + // k8s namespace. For example, if `mirroringK8SPrefix` is set to "k8s-", a + // service in the k8s `staging` namespace will be registered into the + // `k8s-staging` Consul namespace. + NSMirroringPrefix string + decoder *admission.Decoder client.Client } @@ -51,7 +63,9 @@ func (v *ServiceDefaultsWebhook) Handle(ctx context.Context, req admission.Reque v, &svcDefaults, v.EnableConsulNamespaces, - v.EnableNSMirroring) + v.EnableNSMirroring, + v.ConsulDestinationNamespace, + v.NSMirroringPrefix) } func (v *ServiceDefaultsWebhook) List(ctx context.Context) ([]common.ConfigEntryResource, error) { diff --git a/api/v1alpha1/serviceintentions_types.go b/api/v1alpha1/serviceintentions_types.go index 084f9616fd..b58e80944a 100644 --- a/api/v1alpha1/serviceintentions_types.go +++ b/api/v1alpha1/serviceintentions_types.go @@ -271,13 +271,14 @@ func (in *ServiceIntentions) Validate(namespacesEnabled bool) error { return nil } -// Default sets the namespace field on spec.destination to their default values if namespaces are enabled. -func (in *ServiceIntentions) Default(consulNamespacesEnabled bool, destinationNamespace string, mirroring bool, prefix string) { +// DefaultNamespaceFields sets the namespace field on spec.destination to their default values if namespaces are enabled. +func (in *ServiceIntentions) DefaultNamespaceFields(consulNamespacesEnabled bool, destinationNamespace string, mirroring bool, prefix string) { // If namespaces are enabled we want to set the destination namespace field to it's // default. If namespaces are not enabled (i.e. OSS) we don't set the // namespace fields because this would cause errors // making API calls (because namespace fields can't be set in OSS). if consulNamespacesEnabled { + // Default to the current namespace (i.e. the namespace of the config entry). namespace := namespaces.ConsulNamespace(in.Namespace, consulNamespacesEnabled, destinationNamespace, mirroring, prefix) if in.Spec.Destination.Namespace == "" { in.Spec.Destination.Namespace = namespace diff --git a/api/v1alpha1/serviceintentions_types_test.go b/api/v1alpha1/serviceintentions_types_test.go index fc563a9c38..baf8ba6837 100644 --- a/api/v1alpha1/serviceintentions_types_test.go +++ b/api/v1alpha1/serviceintentions_types_test.go @@ -503,7 +503,7 @@ func TestServiceIntentions_ObjectMeta(t *testing.T) { } // Test defaulting behavior when namespaces are enabled as well as disabled. -func TestServiceIntentions_Default(t *testing.T) { +func TestServiceIntentions_DefaultNamespaceFields(t *testing.T) { namespaceConfig := map[string]struct { enabled bool destinationNamespace string @@ -542,32 +542,31 @@ func TestServiceIntentions_Default(t *testing.T) { } for name, s := range namespaceConfig { - input := &ServiceIntentions{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: ServiceIntentionsSpec{ - Destination: Destination{ - Name: "bar", + t.Run(name, func(t *testing.T) { + input := &ServiceIntentions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", }, - }, - } - output := &ServiceIntentions{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: ServiceIntentionsSpec{ - Destination: Destination{ - Name: "bar", - Namespace: s.expectedDestination, + Spec: ServiceIntentionsSpec{ + Destination: Destination{ + Name: "bar", + }, }, - }, - } - - t.Run(name, func(t *testing.T) { - input.Default(s.enabled, s.destinationNamespace, s.mirroring, s.prefix) + } + output := &ServiceIntentions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: ServiceIntentionsSpec{ + Destination: Destination{ + Name: "bar", + Namespace: s.expectedDestination, + }, + }, + } + input.DefaultNamespaceFields(s.enabled, s.destinationNamespace, s.mirroring, s.prefix) require.True(t, cmp.Equal(input, output)) }) } diff --git a/api/v1alpha1/serviceintentions_webhook.go b/api/v1alpha1/serviceintentions_webhook.go index e46797e715..8dfeb9d8d4 100644 --- a/api/v1alpha1/serviceintentions_webhook.go +++ b/api/v1alpha1/serviceintentions_webhook.go @@ -2,14 +2,13 @@ package v1alpha1 import ( "context" - "encoding/json" "errors" "fmt" "net/http" "github.com/go-logr/logr" + "github.com/hashicorp/consul-k8s/api/common" capi "github.com/hashicorp/consul/api" - "gomodules.xyz/jsonpatch/v2" "k8s.io/api/admission/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -45,7 +44,7 @@ func (v *ServiceIntentionsWebhook) Handle(ctx context.Context, req admission.Req return admission.Errored(http.StatusBadRequest, err) } - defaultingPatches, err := v.defaultingPatches(err, &svcIntentions) + defaultingPatches, err := common.DefaultingPatches(&svcIntentions, v.EnableConsulNamespaces, v.EnableNSMirroring, v.ConsulDestinationNamespace, v.NSMirroringPrefix) if err != nil { return admission.Errored(http.StatusInternalServerError, err) } @@ -104,23 +103,3 @@ func (v *ServiceIntentionsWebhook) InjectDecoder(d *admission.Decoder) error { v.decoder = d return nil } - -// defaultingPatches returns the patches needed to set fields to their -// defaults. -func (v *ServiceIntentionsWebhook) defaultingPatches(err error, svcIntentions *ServiceIntentions) ([]jsonpatch.Operation, error) { - beforeDefaulting, err := json.Marshal(svcIntentions) - if err != nil { - return nil, fmt.Errorf("marshalling input: %s", err) - } - svcIntentions.Default(v.EnableConsulNamespaces, v.ConsulDestinationNamespace, v.EnableNSMirroring, v.NSMirroringPrefix) - afterDefaulting, err := json.Marshal(svcIntentions) - if err != nil { - return nil, fmt.Errorf("marshalling after defaulting: %s", err) - } - - defaultingPatches, err := jsonpatch.CreatePatch(beforeDefaulting, afterDefaulting) - if err != nil { - return nil, fmt.Errorf("creating patches: %s", err) - } - return defaultingPatches, nil -} diff --git a/api/v1alpha1/serviceresolver_types.go b/api/v1alpha1/serviceresolver_types.go index dc1df226d8..d2d9ce30ca 100644 --- a/api/v1alpha1/serviceresolver_types.go +++ b/api/v1alpha1/serviceresolver_types.go @@ -306,6 +306,12 @@ func (in *ServiceResolver) Validate(namespacesEnabled bool) error { return nil } +// DefaultNamespaceFields has no behaviour here as service-resolver have namespace fields +// that do not default. +func (in *ServiceResolver) DefaultNamespaceFields(_ bool, _ string, _ bool, _ string) { + return +} + func (in ServiceResolverSubsetMap) toConsul() map[string]capi.ServiceResolverSubset { if in == nil { return nil diff --git a/api/v1alpha1/serviceresolver_webhook.go b/api/v1alpha1/serviceresolver_webhook.go index 7b80e6af09..806b777646 100644 --- a/api/v1alpha1/serviceresolver_webhook.go +++ b/api/v1alpha1/serviceresolver_webhook.go @@ -26,6 +26,18 @@ type ServiceResolverWebhook struct { // be created in the matching Consul namespace. EnableNSMirroring bool + // ConsulDestinationNamespace is the namespace in Consul that the config entry created + // in k8s will get mapped into. If the Consul namespace does not already exist, it will + // be created. + ConsulDestinationNamespace string + + // NSMirroringPrefix works in conjunction with Namespace Mirroring. + // It is the prefix added to the Consul namespace to map to a specific. + // k8s namespace. For example, if `mirroringK8SPrefix` is set to "k8s-", a + // service in the k8s `staging` namespace will be registered into the + // `k8s-staging` Consul namespace. + NSMirroringPrefix string + decoder *admission.Decoder client.Client } @@ -51,7 +63,9 @@ func (v *ServiceResolverWebhook) Handle(ctx context.Context, req admission.Reque v, &svcResolver, v.EnableConsulNamespaces, - v.EnableNSMirroring) + v.EnableNSMirroring, + v.ConsulDestinationNamespace, + v.NSMirroringPrefix) } func (v *ServiceResolverWebhook) List(ctx context.Context) ([]common.ConfigEntryResource, error) { diff --git a/api/v1alpha1/servicerouter_types.go b/api/v1alpha1/servicerouter_types.go index 0571e3aa47..c08042b7af 100644 --- a/api/v1alpha1/servicerouter_types.go +++ b/api/v1alpha1/servicerouter_types.go @@ -6,6 +6,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/consul-k8s/namespaces" capi "github.com/hashicorp/consul/api" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -255,6 +256,25 @@ func (in *ServiceRouter) Validate(namespacesEnabled bool) error { return nil } +// DefaultNamespaceFields sets the namespace field on spec.routes[].destination to their default values if namespaces are enabled. +func (in *ServiceRouter) DefaultNamespaceFields(consulNamespacesEnabled bool, destinationNamespace string, mirroring bool, prefix string) { + // If namespaces are enabled we want to set the namespace fields to their + // defaults. If namespaces are not enabled (i.e. OSS) we don't set the + // namespace fields because this would cause errors + // making API calls (because namespace fields can't be set in OSS). + if consulNamespacesEnabled { + // Default to the current namespace (i.e. the namespace of the config entry). + namespace := namespaces.ConsulNamespace(in.Namespace, consulNamespacesEnabled, destinationNamespace, mirroring, prefix) + for i, r := range in.Spec.Routes { + if r.Destination != nil { + if r.Destination.Namespace == "" { + in.Spec.Routes[i].Destination.Namespace = namespace + } + } + } + } +} + func (in ServiceRoute) toConsul() capi.ServiceRoute { return capi.ServiceRoute{ Match: in.Match.toConsul(), diff --git a/api/v1alpha1/servicerouter_types_test.go b/api/v1alpha1/servicerouter_types_test.go index be320e7626..00efa1c44f 100644 --- a/api/v1alpha1/servicerouter_types_test.go +++ b/api/v1alpha1/servicerouter_types_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/consul-k8s/api/common" capi "github.com/hashicorp/consul/api" "github.com/stretchr/testify/require" @@ -598,3 +599,113 @@ func TestServiceRouter_Validate(t *testing.T) { }) } } + +// Test defaulting behavior when namespaces are enabled as well as disabled. +func TestServiceRouter_DefaultNamespaceFields(t *testing.T) { + namespaceConfig := map[string]struct { + enabled bool + destinationNamespace string + mirroring bool + prefix string + expectedDestination string + }{ + "disabled": { + enabled: false, + destinationNamespace: "", + mirroring: false, + prefix: "", + expectedDestination: "", + }, + "destinationNS": { + enabled: true, + destinationNamespace: "foo", + mirroring: false, + prefix: "", + expectedDestination: "foo", + }, + "mirroringEnabledWithoutPrefix": { + enabled: true, + destinationNamespace: "", + mirroring: true, + prefix: "", + expectedDestination: "bar", + }, + "mirroringWithPrefix": { + enabled: true, + destinationNamespace: "", + mirroring: true, + prefix: "ns-", + expectedDestination: "ns-bar", + }, + } + + for name, s := range namespaceConfig { + t.Run(name, func(t *testing.T) { + input := &ServiceRouter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: ServiceRouterSpec{ + Routes: []ServiceRoute{ + { + Match: &ServiceRouteMatch{ + HTTP: &ServiceRouteHTTPMatch{ + PathPrefix: "/admin", + }, + }, + Destination: &ServiceRouteDestination{ + Service: "destA", + }, + }, + { + Match: &ServiceRouteMatch{ + HTTP: &ServiceRouteHTTPMatch{ + PathPrefix: "/other", + }, + }, + Destination: &ServiceRouteDestination{ + Service: "destB", + Namespace: "other", + }, + }, + }, + }, + } + output := &ServiceRouter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: ServiceRouterSpec{ + Routes: []ServiceRoute{ + { + Match: &ServiceRouteMatch{ + HTTP: &ServiceRouteHTTPMatch{ + PathPrefix: "/admin", + }, + }, + Destination: &ServiceRouteDestination{ + Service: "destA", + Namespace: s.expectedDestination, + }, + }, + { + Match: &ServiceRouteMatch{ + HTTP: &ServiceRouteHTTPMatch{ + PathPrefix: "/other", + }, + }, + Destination: &ServiceRouteDestination{ + Service: "destB", + Namespace: "other", + }, + }, + }, + }, + } + input.DefaultNamespaceFields(s.enabled, s.destinationNamespace, s.mirroring, s.prefix) + require.True(t, cmp.Equal(input, output)) + }) + } +} diff --git a/api/v1alpha1/servicerouter_webhook.go b/api/v1alpha1/servicerouter_webhook.go index 4719b216f9..3c7ffb83a5 100644 --- a/api/v1alpha1/servicerouter_webhook.go +++ b/api/v1alpha1/servicerouter_webhook.go @@ -26,6 +26,18 @@ type ServiceRouterWebhook struct { // be created in the matching Consul namespace. EnableNSMirroring bool + // ConsulDestinationNamespace is the namespace in Consul that the config entry created + // in k8s will get mapped into. If the Consul namespace does not already exist, it will + // be created. + ConsulDestinationNamespace string + + // NSMirroringPrefix works in conjunction with Namespace Mirroring. + // It is the prefix added to the Consul namespace to map to a specific. + // k8s namespace. For example, if `mirroringK8SPrefix` is set to "k8s-", a + // service in the k8s `staging` namespace will be registered into the + // `k8s-staging` Consul namespace. + NSMirroringPrefix string + decoder *admission.Decoder client.Client } @@ -51,7 +63,9 @@ func (v *ServiceRouterWebhook) Handle(ctx context.Context, req admission.Request v, &svcRouter, v.EnableConsulNamespaces, - v.EnableNSMirroring) + v.EnableNSMirroring, + v.ConsulDestinationNamespace, + v.NSMirroringPrefix) } func (v *ServiceRouterWebhook) List(ctx context.Context) ([]common.ConfigEntryResource, error) { diff --git a/api/v1alpha1/servicesplitter_types.go b/api/v1alpha1/servicesplitter_types.go index f6224c25b3..8c4379d826 100644 --- a/api/v1alpha1/servicesplitter_types.go +++ b/api/v1alpha1/servicesplitter_types.go @@ -170,6 +170,12 @@ func (in *ServiceSplitter) Validate(namespacesEnabled bool) error { return nil } +// DefaultNamespaceFields has no behaviour here as service-splitter have namespace fields +// that do not default. +func (in *ServiceSplitter) DefaultNamespaceFields(_ bool, _ string, _ bool, _ string) { + return +} + func (in ServiceSplits) toConsul() []capi.ServiceSplit { var consulServiceSplits []capi.ServiceSplit for _, split := range in { @@ -196,7 +202,6 @@ func (in *ServiceSplitter) validateNamespaces(namespacesEnabled bool) field.Erro if s.Namespace != "" { errs = append(errs, field.Invalid(path.Child("splits").Index(i).Child("namespace"), s.Namespace, `Consul Enterprise namespaces must be enabled to set split.namespace`)) } - } } return errs diff --git a/api/v1alpha1/servicesplitter_webhook.go b/api/v1alpha1/servicesplitter_webhook.go index 7d31b0510e..bcda641bd7 100644 --- a/api/v1alpha1/servicesplitter_webhook.go +++ b/api/v1alpha1/servicesplitter_webhook.go @@ -26,6 +26,18 @@ type ServiceSplitterWebhook struct { // be created in the matching Consul namespace. EnableNSMirroring bool + // ConsulDestinationNamespace is the namespace in Consul that the config entry created + // in k8s will get mapped into. If the Consul namespace does not already exist, it will + // be created. + ConsulDestinationNamespace string + + // NSMirroringPrefix works in conjunction with Namespace Mirroring. + // It is the prefix added to the Consul namespace to map to a specific. + // k8s namespace. For example, if `mirroringK8SPrefix` is set to "k8s-", a + // service in the k8s `staging` namespace will be registered into the + // `k8s-staging` Consul namespace. + NSMirroringPrefix string + decoder *admission.Decoder client.Client } @@ -52,7 +64,9 @@ func (v *ServiceSplitterWebhook) Handle(ctx context.Context, req admission.Reque v, &serviceSplitter, v.EnableConsulNamespaces, - v.EnableNSMirroring) + v.EnableNSMirroring, + v.ConsulDestinationNamespace, + v.NSMirroringPrefix) } func (v *ServiceSplitterWebhook) List(ctx context.Context) ([]common.ConfigEntryResource, error) { diff --git a/api/v1alpha1/terminatinggateway_types.go b/api/v1alpha1/terminatinggateway_types.go index 0c5bd379a5..fa4e47eb8d 100644 --- a/api/v1alpha1/terminatinggateway_types.go +++ b/api/v1alpha1/terminatinggateway_types.go @@ -5,6 +5,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/consul-k8s/namespaces" capi "github.com/hashicorp/consul/api" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -188,6 +189,23 @@ func (in *TerminatingGateway) Validate(namespacesEnabled bool) error { return nil } +// DefaultNamespaceFields sets the namespace field on spec.services to their default values if namespaces are enabled. +func (in *TerminatingGateway) DefaultNamespaceFields(consulNamespacesEnabled bool, destinationNamespace string, mirroring bool, prefix string) { + // If namespaces are enabled we want to set the namespace fields to their + // defaults. If namespaces are not enabled (i.e. OSS) we don't set the + // namespace fields because this would cause errors + // making API calls (because namespace fields can't be set in OSS). + if consulNamespacesEnabled { + // Default to the current namespace (i.e. the namespace of the config entry). + namespace := namespaces.ConsulNamespace(in.Namespace, consulNamespacesEnabled, destinationNamespace, mirroring, prefix) + for i, service := range in.Spec.Services { + if service.Namespace == "" { + in.Spec.Services[i].Namespace = namespace + } + } + } +} + func (in LinkedService) toConsul() capi.LinkedService { return capi.LinkedService{ Namespace: in.Namespace, diff --git a/api/v1alpha1/terminatinggateway_types_test.go b/api/v1alpha1/terminatinggateway_types_test.go index 51edc2521b..e659ee28b9 100644 --- a/api/v1alpha1/terminatinggateway_types_test.go +++ b/api/v1alpha1/terminatinggateway_types_test.go @@ -3,6 +3,7 @@ package v1alpha1 import ( "testing" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/consul-k8s/api/common" capi "github.com/hashicorp/consul/api" "github.com/stretchr/testify/require" @@ -279,6 +280,88 @@ func TestTerminatingGateway_Validate(t *testing.T) { } } +// Test defaulting behavior when namespaces are enabled as well as disabled. +func TestTerminatingGateway_DefaultNamespaceFields(t *testing.T) { + namespaceConfig := map[string]struct { + enabled bool + destinationNamespace string + mirroring bool + prefix string + expectedDestination string + }{ + "disabled": { + enabled: false, + destinationNamespace: "", + mirroring: false, + prefix: "", + expectedDestination: "", + }, + "destinationNS": { + enabled: true, + destinationNamespace: "foo", + mirroring: false, + prefix: "", + expectedDestination: "foo", + }, + "mirroringEnabledWithoutPrefix": { + enabled: true, + destinationNamespace: "", + mirroring: true, + prefix: "", + expectedDestination: "bar", + }, + "mirroringWithPrefix": { + enabled: true, + destinationNamespace: "", + mirroring: true, + prefix: "ns-", + expectedDestination: "ns-bar", + }, + } + + for name, s := range namespaceConfig { + t.Run(name, func(t *testing.T) { + input := &TerminatingGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: TerminatingGatewaySpec{ + Services: []LinkedService{ + { + Name: "foo", + }, + { + Name: "bar", + Namespace: "other", + }, + }, + }, + } + output := &TerminatingGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: TerminatingGatewaySpec{ + Services: []LinkedService{ + { + Name: "foo", + Namespace: s.expectedDestination, + }, + { + Name: "bar", + Namespace: "other", + }, + }, + }, + } + input.DefaultNamespaceFields(s.enabled, s.destinationNamespace, s.mirroring, s.prefix) + require.True(t, cmp.Equal(input, output)) + }) + } +} + func TestTerminatingGateway_AddFinalizer(t *testing.T) { resource := &TerminatingGateway{} resource.AddFinalizer("finalizer") diff --git a/api/v1alpha1/terminatinggateway_webhook.go b/api/v1alpha1/terminatinggateway_webhook.go index 59b3d91e5e..efde0a6f44 100644 --- a/api/v1alpha1/terminatinggateway_webhook.go +++ b/api/v1alpha1/terminatinggateway_webhook.go @@ -26,6 +26,18 @@ type TerminatingGatewayWebhook struct { // be created in the matching Consul namespace. EnableNSMirroring bool + // ConsulDestinationNamespace is the namespace in Consul that the config entry created + // in k8s will get mapped into. If the Consul namespace does not already exist, it will + // be created. + ConsulDestinationNamespace string + + // NSMirroringPrefix works in conjunction with Namespace Mirroring. + // It is the prefix added to the Consul namespace to map to a specific. + // k8s namespace. For example, if `mirroringK8SPrefix` is set to "k8s-", a + // service in the k8s `staging` namespace will be registered into the + // `k8s-staging` Consul namespace. + NSMirroringPrefix string + decoder *admission.Decoder client.Client } @@ -51,7 +63,9 @@ func (v *TerminatingGatewayWebhook) Handle(ctx context.Context, req admission.Re v, &resource, v.EnableConsulNamespaces, - v.EnableNSMirroring) + v.EnableNSMirroring, + v.ConsulDestinationNamespace, + v.NSMirroringPrefix) } func (v *TerminatingGatewayWebhook) List(ctx context.Context) ([]common.ConfigEntryResource, error) { diff --git a/subcommand/controller/command.go b/subcommand/controller/command.go index de0aa5ab61..5dc859e166 100644 --- a/subcommand/controller/command.go +++ b/subcommand/controller/command.go @@ -226,19 +226,23 @@ func (c *Command) Run(args []string) int { // annotation in each webhook file. mgr.GetWebhookServer().Register("/mutate-v1alpha1-servicedefaults", &webhook.Admission{Handler: &v1alpha1.ServiceDefaultsWebhook{ - Client: mgr.GetClient(), - ConsulClient: consulClient, - Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceDefaults), - EnableConsulNamespaces: c.flagEnableNamespaces, - EnableNSMirroring: c.flagEnableNSMirroring, + Client: mgr.GetClient(), + ConsulClient: consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceDefaults), + EnableConsulNamespaces: c.flagEnableNamespaces, + EnableNSMirroring: c.flagEnableNSMirroring, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + NSMirroringPrefix: c.flagNSMirroringPrefix, }}) mgr.GetWebhookServer().Register("/mutate-v1alpha1-serviceresolver", &webhook.Admission{Handler: &v1alpha1.ServiceResolverWebhook{ - Client: mgr.GetClient(), - ConsulClient: consulClient, - Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceResolver), - EnableConsulNamespaces: c.flagEnableNamespaces, - EnableNSMirroring: c.flagEnableNSMirroring, + Client: mgr.GetClient(), + ConsulClient: consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceResolver), + EnableConsulNamespaces: c.flagEnableNamespaces, + EnableNSMirroring: c.flagEnableNSMirroring, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + NSMirroringPrefix: c.flagNSMirroringPrefix, }}) mgr.GetWebhookServer().Register("/mutate-v1alpha1-proxydefaults", &webhook.Admission{Handler: &v1alpha1.ProxyDefaultsWebhook{ @@ -250,19 +254,23 @@ func (c *Command) Run(args []string) int { }}) mgr.GetWebhookServer().Register("/mutate-v1alpha1-servicerouter", &webhook.Admission{Handler: &v1alpha1.ServiceRouterWebhook{ - Client: mgr.GetClient(), - ConsulClient: consulClient, - Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceRouter), - EnableConsulNamespaces: c.flagEnableNamespaces, - EnableNSMirroring: c.flagEnableNSMirroring, + Client: mgr.GetClient(), + ConsulClient: consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceRouter), + EnableConsulNamespaces: c.flagEnableNamespaces, + EnableNSMirroring: c.flagEnableNSMirroring, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + NSMirroringPrefix: c.flagNSMirroringPrefix, }}) mgr.GetWebhookServer().Register("/mutate-v1alpha1-servicesplitter", &webhook.Admission{Handler: &v1alpha1.ServiceSplitterWebhook{ - Client: mgr.GetClient(), - ConsulClient: consulClient, - Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceSplitter), - EnableConsulNamespaces: c.flagEnableNamespaces, - EnableNSMirroring: c.flagEnableNSMirroring, + Client: mgr.GetClient(), + ConsulClient: consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName(common.ServiceSplitter), + EnableConsulNamespaces: c.flagEnableNamespaces, + EnableNSMirroring: c.flagEnableNSMirroring, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + NSMirroringPrefix: c.flagNSMirroringPrefix, }}) mgr.GetWebhookServer().Register("/mutate-v1alpha1-serviceintentions", &webhook.Admission{Handler: &v1alpha1.ServiceIntentionsWebhook{ @@ -276,19 +284,23 @@ func (c *Command) Run(args []string) int { }}) mgr.GetWebhookServer().Register("/mutate-v1alpha1-ingressgateway", &webhook.Admission{Handler: &v1alpha1.IngressGatewayWebhook{ - Client: mgr.GetClient(), - ConsulClient: consulClient, - Logger: ctrl.Log.WithName("webhooks").WithName(common.IngressGateway), - EnableConsulNamespaces: c.flagEnableNamespaces, - EnableNSMirroring: c.flagEnableNSMirroring, + Client: mgr.GetClient(), + ConsulClient: consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName(common.IngressGateway), + EnableConsulNamespaces: c.flagEnableNamespaces, + EnableNSMirroring: c.flagEnableNSMirroring, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + NSMirroringPrefix: c.flagNSMirroringPrefix, }}) mgr.GetWebhookServer().Register("/mutate-v1alpha1-terminatinggateway", &webhook.Admission{Handler: &v1alpha1.TerminatingGatewayWebhook{ - Client: mgr.GetClient(), - ConsulClient: consulClient, - Logger: ctrl.Log.WithName("webhooks").WithName(common.TerminatingGateway), - EnableConsulNamespaces: c.flagEnableNamespaces, - EnableNSMirroring: c.flagEnableNSMirroring, + Client: mgr.GetClient(), + ConsulClient: consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName(common.TerminatingGateway), + EnableConsulNamespaces: c.flagEnableNamespaces, + EnableNSMirroring: c.flagEnableNSMirroring, + ConsulDestinationNamespace: c.flagConsulDestinationNamespace, + NSMirroringPrefix: c.flagNSMirroringPrefix, }}) } // +kubebuilder:scaffold:builder