Default namespaces for CRDs that support namespace when enabled

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)
 

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)
 }

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
 }

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
 }

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,

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")

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) {

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.

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)

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) {

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