Include cluster name in k8s Service Account name for Workload Identity (

#950)

* update configure

* Revert "update configure"

This reverts commit fa4c9bb

* add cluster name

* update code

* update code

* update unit test

* code update

* ut

* update comment

* update-codegen

* solve conflict/ut

* solve conflict/ut

cmd/broker/fanout/main.go

@@ -21,11 +21,14 @@ import (
 	"time"
 
 	"cloud.google.com/go/pubsub"
+
 	"github.com/google/knative-gcp/pkg/broker/config/volume"
 	"github.com/google/knative-gcp/pkg/broker/handler/pool"
+	metadataClient "github.com/google/knative-gcp/pkg/gclient/metadata"
 	"github.com/google/knative-gcp/pkg/utils"
 	"github.com/google/knative-gcp/pkg/utils/appcredentials"
 	"github.com/google/knative-gcp/pkg/utils/mainhelper"
+
 	"go.uber.org/zap"
 )
 
@@ -58,7 +61,7 @@ func main() {
 
 	logger.Info("Starting the broker fanout")
 
-	projectID, err := utils.ProjectID(env.ProjectID)
+	projectID, err := utils.ProjectID(env.ProjectID, metadataClient.NewDefaultMetadataClient())
 	if err != nil {
 		logger.Fatalf("failed to get default ProjectID: %v", err)
 	}

cmd/broker/ingress/main.go

@@ -18,10 +18,12 @@ package main
 
 import (
 	"github.com/google/knative-gcp/pkg/broker/ingress"
+	metadataClient "github.com/google/knative-gcp/pkg/gclient/metadata"
 	"github.com/google/knative-gcp/pkg/metrics"
 	"github.com/google/knative-gcp/pkg/utils"
 	"github.com/google/knative-gcp/pkg/utils/appcredentials"
 	"github.com/google/knative-gcp/pkg/utils/mainhelper"
+
 	"go.uber.org/zap"
 )
 
@@ -50,7 +52,7 @@ func main() {
 	defer res.Cleanup()
 	logger := res.Logger
 
-	projectID, err := utils.ProjectID(env.ProjectID)
+	projectID, err := utils.ProjectID(env.ProjectID, metadataClient.NewDefaultMetadataClient())
 	if err != nil {
 		logger.Desugar().Fatal("Failed to create project id", zap.Error(err))
 	}

cmd/broker/retry/main.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/google/knative-gcp/pkg/broker/config/volume"
 	"github.com/google/knative-gcp/pkg/broker/handler/pool"
+	metadataClient "github.com/google/knative-gcp/pkg/gclient/metadata"
 	"github.com/google/knative-gcp/pkg/utils"
 	"github.com/google/knative-gcp/pkg/utils/appcredentials"
 	"github.com/google/knative-gcp/pkg/utils/mainhelper"
@@ -61,7 +62,7 @@ func main() {
 	targetsUpdateCh := make(chan struct{})
 	logger.Info("Starting the broker retry")
 
-	projectID, err := utils.ProjectID(env.ProjectID)
+	projectID, err := utils.ProjectID(env.ProjectID, metadataClient.NewDefaultMetadataClient())
 	if err != nil {
 		logger.Fatalf("failed to get default ProjectID: %v", err)
 	}

hack/tools.go

@@ -27,8 +27,8 @@ import (
 	_ "knative.dev/eventing/test/test_images/sendevents"
 	_ "knative.dev/eventing/test/test_images/transformevents"
 
-	_ "knative.dev/pkg/testutils/clustermanager/perf-tests"
 	_ "knative.dev/eventing/test/test_images/performance"
+	_ "knative.dev/pkg/testutils/clustermanager/perf-tests"
 
 	_ "github.com/google/wire/cmd/wire"
 )

pkg/apis/duck/v1alpha1/annotations.go

@@ -22,8 +22,12 @@ import (
 	"math"
 	"strconv"
 
+	"github.com/google/go-cmp/cmp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
+
+	metadataClient "github.com/google/knative-gcp/pkg/gclient/metadata"
+	"github.com/google/knative-gcp/pkg/utils"
 )
 
 const (
@@ -33,6 +37,8 @@ const (
 	// AutoscalingClassAnnotation is the annotation for the explicit class of
 	// scaler that a particular resource has opted into.
 	AutoscalingClassAnnotation = Autoscaling + "/class"
+	// ClusterNameAnnotation is the annotation for the cluster Name.
+	ClusterNameAnnotation = "cluster-name"
 
 	// AutoscalingMinScaleAnnotation is the annotation to specify the minimum number of pods to scale to.
 	AutoscalingMinScaleAnnotation = Autoscaling + "/minScale"
@@ -144,6 +150,9 @@ func validateAnnotation(annotations map[string]string, annotation string, minimu
 }
 
 func setDefaultAnnotationIfNotPresent(obj *metav1.ObjectMeta, annotation string, defaultValue string) {
+	if obj.Annotations == nil {
+		obj.Annotations = map[string]string{}
+	}
 	if _, ok := obj.Annotations[annotation]; !ok {
 		obj.Annotations[annotation] = defaultValue
 	}
@@ -161,3 +170,28 @@ func validateAnnotationNotExists(annotations map[string]string, annotation strin
 	}
 	return errs
 }
+
+// SetClusterNameAnnotation sets the cluster-name annotation when running on GKE or GCE.
+func SetClusterNameAnnotation(obj *metav1.ObjectMeta, client metadataClient.Client) {
+	if _, ok := obj.Annotations[ClusterNameAnnotation]; !ok && client.OnGCE() {
+		clusterName, err := utils.ClusterName(obj.Annotations[ClusterNameAnnotation], client)
+		// If metadata access is disabled for some reason, leave the annotation to be empty.
+		if err == nil {
+			setDefaultAnnotationIfNotPresent(obj, ClusterNameAnnotation, clusterName)
+		}
+	}
+}
+
+// CheckImmutableClusterNameAnnotation checks non-empty cluster-name annotation is immutable.
+func CheckImmutableClusterNameAnnotation(current *metav1.ObjectMeta, original *metav1.ObjectMeta, errs *apis.FieldError) *apis.FieldError {
+	if _, ok := original.Annotations[ClusterNameAnnotation]; ok {
+		if diff := cmp.Diff(original.Annotations[ClusterNameAnnotation], current.Annotations[ClusterNameAnnotation]); diff != "" {
+			return errs.Also(&apis.FieldError{
+				Message: "Immutable fields changed (-old +new)",
+				Paths:   []string{fmt.Sprintf("metadata.annotations[%s]", ClusterNameAnnotation)},
+				Details: diff,
+			})
+		}
+	}
+	return errs
+}

pkg/apis/duck/v1alpha1/annotations_test.go

@@ -18,11 +18,14 @@ package v1alpha1
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
+
+	testingMetadataClient "github.com/google/knative-gcp/pkg/gclient/metadata/testing"
 )
 
 var (
@@ -208,3 +211,102 @@ func TestValidateAutoscalingAnnotations(t *testing.T) {
 		})
 	}
 }
+
+func TestSetClusterNameAnnotation(t *testing.T) {
+	testCases := map[string]struct {
+		orig     *v1.ObjectMeta
+		data     testingMetadataClient.TestClientData
+		expected *v1.ObjectMeta
+	}{
+		"no annotation, successfully get the clusterName": {
+			orig: &v1.ObjectMeta{},
+			data: testingMetadataClient.TestClientData{},
+			expected: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
+				},
+			},
+		},
+		"no annotation, get clusterName failed": {
+			orig: &v1.ObjectMeta{},
+			data: testingMetadataClient.TestClientData{
+				ClusterNameErr: fmt.Errorf("error when get clusterName"),
+			},
+			expected: &v1.ObjectMeta{},
+		},
+		"has annotation": {
+			orig: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: "testing-cluster-name",
+				},
+			},
+			data: testingMetadataClient.TestClientData{},
+			expected: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: "testing-cluster-name",
+				},
+			},
+		},
+	}
+	for n, tc := range testCases {
+		t.Run(n, func(t *testing.T) {
+			SetClusterNameAnnotation(tc.orig, testingMetadataClient.NewTestClient(tc.data))
+			if diff := cmp.Diff(tc.expected, tc.orig); diff != "" {
+				t.Errorf("Unexpected differences (-want +got): %v", diff)
+			}
+		})
+	}
+}
+
+func TestCheckImmutableClusterNameAnnotation(t *testing.T) {
+	testCases := map[string]struct {
+		original *v1.ObjectMeta
+		current  *v1.ObjectMeta
+		error    bool
+	}{
+		"update empty annotation": {
+			original: &v1.ObjectMeta{},
+			current: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
+				},
+			},
+			error: false,
+		},
+		"update non-empty annotation": {
+			original: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: testingMetadataClient.FakeClusterName + "old",
+				},
+			},
+			current: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: testingMetadataClient.FakeClusterName + "new",
+				},
+			},
+			error: true,
+		},
+		"unchanged annotation": {
+			original: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: "testing-cluster-name",
+				},
+			},
+			current: &v1.ObjectMeta{
+				Annotations: map[string]string{
+					ClusterNameAnnotation: "testing-cluster-name",
+				},
+			},
+			error: false,
+		},
+	}
+	for n, tc := range testCases {
+		t.Run(n, func(t *testing.T) {
+			var err *apis.FieldError
+			err = CheckImmutableClusterNameAnnotation(tc.current, tc.original, err)
+			if tc.error != (err != nil) {
+				t.Fatalf("Unexpected validation failure. Got %v", err)
+			}
+		})
+	}
+}

pkg/apis/events/v1alpha1/cloudauditlogssource_defaults.go

@@ -20,9 +20,11 @@ import (
 	"context"
 
 	duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1"
+	metadataClient "github.com/google/knative-gcp/pkg/gclient/metadata"
 )
 
 func (s *CloudAuditLogsSource) SetDefaults(ctx context.Context) {
 	s.Spec.SetPubSubDefaults()
+	duckv1alpha1.SetClusterNameAnnotation(&s.ObjectMeta, metadataClient.NewDefaultMetadataClient())
 	duckv1alpha1.SetAutoscalingAnnotationsDefaults(ctx, &s.ObjectMeta)
 }

pkg/apis/events/v1alpha1/cloudauditlogssource_defaults_test.go

@@ -22,17 +22,32 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1"
+	testingMetadataClient "github.com/google/knative-gcp/pkg/gclient/metadata/testing"
+
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestCloudAuditLogsSource_SetDefaults(t *testing.T) {
 	testCases := map[string]struct {
 		orig     *CloudAuditLogsSource
 		expected *CloudAuditLogsSource
 	}{
-		"missing defaults": {
-			orig: &CloudAuditLogsSource{},
+		// Due to the limitation mentioned in https://github.com/google/knative-gcp/issues/1037, specifying the cluster name annotation.
+		"missing defaults, except cluster name annotations": {
+			orig: &CloudAuditLogsSource{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
+					},
+				},
+			},
 			expected: &CloudAuditLogsSource{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
+					},
+				},
 				Spec: CloudAuditLogsSourceSpec{
 					PubSubSpec: duckv1alpha1.PubSubSpec{
 						Secret: &corev1.SecretKeySelector{
@@ -47,6 +62,11 @@ func TestCloudAuditLogsSource_SetDefaults(t *testing.T) {
 		},
 		"defaults present": {
 			orig: &CloudAuditLogsSource{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
+					},
+				},
 				Spec: CloudAuditLogsSourceSpec{
 					PubSubSpec: duckv1alpha1.PubSubSpec{
 						Secret: &corev1.SecretKeySelector{
@@ -59,6 +79,11 @@ func TestCloudAuditLogsSource_SetDefaults(t *testing.T) {
 				},
 			},
 			expected: &CloudAuditLogsSource{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
+					},
+				},
 				Spec: CloudAuditLogsSourceSpec{
 					PubSubSpec: duckv1alpha1.PubSubSpec{
 						Secret: &corev1.SecretKeySelector{

pkg/apis/events/v1alpha1/cloudauditlogssource_validation.go

@@ -21,10 +21,11 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"knative.dev/pkg/apis"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
+
+	duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1"
 )
 
 func (current *CloudAuditLogsSource) Validate(ctx context.Context) *apis.FieldError {
@@ -62,15 +63,18 @@ func (current *CloudAuditLogsSource) CheckImmutableFields(ctx context.Context, o
 		return nil
 	}
 
+	var errs *apis.FieldError
 	// Modification of Topic, Secret, ServiceAccount, Project, ServiceName, MethodName, and ResourceName are not allowed. Everything else is mutable.
 	if diff := cmp.Diff(original.Spec, current.Spec,
 		cmpopts.IgnoreFields(CloudAuditLogsSourceSpec{},
 			"Sink", "CloudEventOverrides")); diff != "" {
-		return &apis.FieldError{
-			Message: "Immutable fields changed (-old +new)",
-			Paths:   []string{"spec"},
-			Details: diff,
-		}
+		errs = errs.Also(
+			&apis.FieldError{
+				Message: "Immutable fields changed (-old +new)",
+				Paths:   []string{"spec"},
+				Details: diff,
+			})
 	}
-	return nil
+	// Modification of non-empty cluster name annotation is not allowed.
+	return duckv1alpha1.CheckImmutableClusterNameAnnotation(&current.ObjectMeta, &original.ObjectMeta, errs)
 }