PR feedback

control-plane/connect-inject/common/common.go

@@ -181,8 +181,8 @@ func PortValueFromIntOrString(pod corev1.Pod, port intstr.IntOrString) (uint32,
 	return uint32(portVal), nil
 }
 
-// ConsulNamespaceIsNotFound checks the gRPC error code and message to determine if the
-// error indicates that the namespace of for a resource does not yet exist.
+// ConsulNamespaceIsNotFound checks the gRPC error code and message to determine
+// if a namespace does not exist. If the namespace exists this function returns false, true otherwise.
 func ConsulNamespaceIsNotFound(err error) bool {
 	if err == nil {
 		return false

control-plane/connect-inject/common/common_test.go

@@ -4,11 +4,13 @@
 package common
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
 	mapset "github.com/deckarep/golang-set"
 	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -21,6 +23,8 @@ import (
 	"github.com/hashicorp/consul/proto-public/pbresource"
 
 	"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
+	"github.com/hashicorp/consul-k8s/control-plane/consul"
+	"github.com/hashicorp/consul-k8s/control-plane/helper/test"
 	"github.com/hashicorp/consul-k8s/control-plane/namespaces"
 )
 
@@ -412,6 +416,10 @@ func Test_ConsulNamespaceIsNotFound(t *testing.T) {
 		input                  error
 		expectMissingNamespace bool
 	}{
+		{
+			name:                   "nil error",
+			expectMissingNamespace: false,
+		},
 		{
 			name:                   "random error",
 			input:                  fmt.Errorf("namespace resource not found"),
@@ -441,3 +449,62 @@ func Test_ConsulNamespaceIsNotFound(t *testing.T) {
 		})
 	}
 }
+
+// Test_ConsulNamespaceIsNotFound_ErrorMsg is an integration test that verifies the error message
+// associated with a missing namespace while creating a resource doesn't drift.
+func Test_ConsulNamespaceIsNotFound_ErrorMsg(t *testing.T) {
+	t.Parallel()
+
+	// Create test consulServer server.
+	testClient := test.TestServerWithMockConnMgrWatcher(t, func(c *testutil.TestServerConfig) {
+		c.Experiments = []string{"resource-apis"}
+	})
+	resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
+	require.NoError(t, err)
+
+	id := &pbresource.ID{
+		Name: "foo",
+		Type: &pbresource.Type{
+			Group:        "catalog",
+			GroupVersion: "v1alpha1",
+			Kind:         "Workload",
+		},
+		Tenancy: &pbresource.Tenancy{
+			Partition: constants.DefaultConsulPartition,
+			Namespace: "i-dont-exist-but-its-ok-we-will-meet-again-someday",
+
+			// Because we are explicitly defining NS/partition, this will not default and must be explicit.
+			// At a future point, this will move out of the Tenancy block.
+			PeerName: constants.DefaultConsulPeer,
+		},
+	}
+
+	workload := &pbcatalog.Workload{
+		Addresses: []*pbcatalog.WorkloadAddress{
+			{Host: "10.0.0.1", Ports: []string{"mesh"}},
+		},
+		Ports: map[string]*pbcatalog.WorkloadPort{
+			"mesh": {
+				Port:     constants.ProxyDefaultInboundPort,
+				Protocol: pbcatalog.Protocol_PROTOCOL_MESH,
+			},
+		},
+		NodeName: "banana",
+		Identity: "foo",
+	}
+
+	data := ToProtoAny(workload)
+
+	resource := &pbresource.Resource{
+		Id:   id,
+		Data: data,
+	}
+
+	_, err = resourceClient.Write(context.Background(), &pbresource.WriteRequest{Resource: resource})
+	require.Error(t, err)
+
+	s, ok := status.FromError(err)
+	require.True(t, ok)
+	require.Equal(t, codes.InvalidArgument, s.Code())
+	require.Contains(t, s.Message(), "namespace resource not found")
+}

control-plane/connect-inject/controllers/endpointsv2/endpoints_controller.go

@@ -125,7 +125,8 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		// Requeue (which includes backoff) to try again.
 		if common.ConsulNamespaceIsNotFound(err) {
 			r.Log.Info("Consul namespace not found; re-queueing request",
-				"service", service.GetName(), "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
+				"service", service.GetName(), "ns", req.Namespace,
+				"consul-ns", r.getConsulNamespace(req.Namespace), "err", err.Error())
 			return ctrl.Result{Requeue: true}, nil
 		}
 		errs = multierror.Append(errs, err)

control-plane/connect-inject/controllers/pod/pod_controller.go

@@ -149,16 +149,20 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			// Requeue (which includes backoff) to try again.
 			if common.ConsulNamespaceIsNotFound(err) {
 				r.Log.Info("Consul namespace not found; re-queueing request",
-					"pod", req.Name, "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
+					"pod", req.Name, "ns", req.Namespace, "consul-ns",
+					r.getConsulNamespace(req.Namespace), "err", err.Error())
 				return ctrl.Result{Requeue: true}, nil
 			}
 			errs = multierror.Append(errs, err)
 		}
 
 		if err := r.writeWorkload(ctx, pod); err != nil {
+			// Technically this is not needed, but keeping in case this gets refactored in
+			// a different order
 			if common.ConsulNamespaceIsNotFound(err) {
 				r.Log.Info("Consul namespace not found; re-queueing request",
-					"pod", req.Name, "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
+					"pod", req.Name, "ns", req.Namespace, "consul-ns",
+					r.getConsulNamespace(req.Namespace), "err", err.Error())
 				return ctrl.Result{Requeue: true}, nil
 			}
 			errs = multierror.Append(errs, err)
@@ -170,9 +174,12 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		//}
 
 		if err := r.writeHealthStatus(ctx, pod); err != nil {
+			// Technically this is not needed, but keeping in case this gets refactored in
+			// a different order
 			if common.ConsulNamespaceIsNotFound(err) {
 				r.Log.Info("Consul namespace not found; re-queueing request",
-					"pod", req.Name, "ns", req.Namespace, "consul-ns", r.getConsulNamespace(req.Namespace))
+					"pod", req.Name, "ns", req.Namespace, "consul-ns",
+					r.getConsulNamespace(req.Namespace), "err", err.Error())
 				return ctrl.Result{Requeue: true}, nil
 			}
 			errs = multierror.Append(errs, err)