Remove spec.googleServiceAccount in v1alpha1 (#1249)

* remove googleServiceAccount * comments
google · Jun 9, 2020 · a12cfb0 · a12cfb0
1 parent af7702b
commit a12cfb0
Show file tree

Hide file tree

Showing 58 changed files with 89 additions and 707 deletions.
diff --git a/config/core/resources/channel.yaml b/config/core/resources/channel.yaml
@@ -72,12 +72,6 @@ spec:
             spec:
               type: object
               properties:
-                googleServiceAccount:
-                  type: string
-                  description: >
-                    GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service
-                    account must be a valid Google service account. (see
-                    https://cloud.google.com/iam/docs/service-accounts).
                 serviceAccountName:
                   type: string
                   description: >

diff --git a/config/core/resources/cloudauditlogssource.yaml b/config/core/resources/cloudauditlogssource.yaml
@@ -110,12 +110,6 @@ spec:
                     Extensions specify what attribute are added or overridden on the outbound event. Each
                     `Extensions` key-value pair are set on the event as an attribute extension independently.
                   x-kubernetes-preserve-unknown-fields: true
-            googleServiceAccount:
-              type: string
-              description: >
-                GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service
-                account must be a valid Google service account. (see
-                https://cloud.google.com/iam/docs/service-accounts).
             serviceAccountName:
               type: string
               description: >

diff --git a/config/core/resources/cloudbuildsource.yaml b/config/core/resources/cloudbuildsource.yaml
@@ -100,12 +100,6 @@ spec:
                     Extensions specify what attribute are added or overridden on the outbound event. Each
                     `Extensions` key-value pair are set on the event as an attribute extension independently.
                   x-kubernetes-preserve-unknown-fields: true
-            googleServiceAccount:
-              type: string
-              description: >
-                GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service
-                account must be a valid Google service account. (see
-                https://cloud.google.com/iam/docs/service-accounts).
             serviceAccountName:
               type: string
               description: >

diff --git a/config/core/resources/cloudpubsubsource.yaml b/config/core/resources/cloudpubsubsource.yaml
@@ -109,12 +109,6 @@ spec:
                     Extensions specify what attribute are added or overridden on the outbound event. Each
                     `Extensions` key-value pair are set on the event as an attribute extension independently.
                   x-kubernetes-preserve-unknown-fields: true
-            googleServiceAccount:
-              type: string
-              description: >
-                GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service
-                account must be a valid Google service account. (see
-                https://cloud.google.com/iam/docs/service-accounts).
             serviceAccountName:
               type: string
               description: >

diff --git a/config/core/resources/cloudschedulersource.yaml b/config/core/resources/cloudschedulersource.yaml
@@ -111,12 +111,6 @@ spec:
                     Extensions specify what attribute are added or overridden on the outbound event. Each
                     `Extensions` key-value pair are set on the event as an attribute extension independently.
                   x-kubernetes-preserve-unknown-fields: true
-            googleServiceAccount:
-              type: string
-              description: >
-                GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service
-                account must be a valid Google service account. (see
-                https://cloud.google.com/iam/docs/service-accounts).
             serviceAccountName:
               type: string
               description: >

diff --git a/config/core/resources/cloudstoragesource.yaml b/config/core/resources/cloudstoragesource.yaml
@@ -112,12 +112,6 @@ spec:
                     Extensions specify what attribute are added or overridden on the outbound event. Each
                     `Extensions` key-value pair are set on the event as an attribute extension independently.
                   x-kubernetes-preserve-unknown-fields: true
-            googleServiceAccount:
-              type: string
-              description: >
-                GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service
-                account must be a valid Google service account. (see
-                https://cloud.google.com/iam/docs/service-accounts).
             serviceAccountName:
               type: string
               description: >

diff --git a/config/core/resources/pullsubscription.yaml b/config/core/resources/pullsubscription.yaml
@@ -67,9 +67,6 @@ spec:
             - sink
             - topic
           properties:
-            googleServiceAccount:
-              type: string
-              description: "GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service account must be a valid Google service account (see https://cloud.google.com/iam/docs/service-accounts)."
             serviceAccountName:
               type: string
               description: "Kubernetes service account used to bind to a google service account to poll the Cloud Pub/Sub Subscription. The value of the Kubernetes service account must be a valid DNS subdomain name. (see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names)"

diff --git a/config/core/resources/topic.yaml b/config/core/resources/topic.yaml
@@ -69,9 +69,6 @@ spec:
           required:
             - topic
           properties:
-            googleServiceAccount:
-              type: string
-              description: "GCP service account used to poll the Cloud Pub/Sub Subscription. The value of the service account must be a valid Google service account (see https://cloud.google.com/iam/docs/service-accounts)."
             serviceAccountName:
               type: string
               description: "Kubernetes service account used to bind to a google service account to poll the Cloud Pub/Sub Subscription. The value of the Kubernetes service account must be a valid DNS subdomain name. (see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names)"

diff --git a/pkg/apis/duck/v1alpha1/credentials.go b/pkg/apis/duck/v1alpha1/credentials.go
@@ -30,37 +30,24 @@ const (
 )
 
 var (
-	// The format of gServiceAccountName is service-account-name@project-id.iam.gserviceaccount.com
-	// Service account name must be between 6 and 30 characters (inclusive),
-	// must begin with a lowercase letter, and consist of lowercase alphanumeric characters that can be separated by hyphens.
-	// Project IDs must start with a lowercase letter and can have lowercase ASCII letters, digits or hyphens,
-	// must be between 6 and 30 characters.
-	gsaValidationRegex = regexp.MustCompile(`^[a-z][a-z0-9-]{5,29}@[a-z][a-z0-9-]{5,29}.iam.gserviceaccount.com$`)
-
 	// The name of a k8s ServiceAccount object must be a valid DNS subdomain name.
 	// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names
 	ksaValidationRegex = regexp.MustCompile(`^[A-Za-z0-9](?:[A-Za-z0-9\-]{0,61}[A-Za-z0-9])?$`)
 )
 
 // ValidateCredential checks secret and service account.
-func ValidateCredential(secret *corev1.SecretKeySelector, gServiceAccountName string, kServiceAccountName string) *apis.FieldError {
+func ValidateCredential(secret *corev1.SecretKeySelector, kServiceAccountName string) *apis.FieldError {
 	if secret != nil && !equality.Semantic.DeepEqual(secret, &corev1.SecretKeySelector{}) && kServiceAccountName != "" {
 		return &apis.FieldError{
 			Message: "Can't have spec.serviceAccountName and spec.secret at the same time",
 			Paths:   []string{""},
 		}
 	} else if secret != nil && !equality.Semantic.DeepEqual(secret, &corev1.SecretKeySelector{}) {
 		return validateSecret(secret)
-	} else {
-		var errs *apis.FieldError
-		if kServiceAccountName != "" {
-			errs = errs.Also(validateK8sServiceAccount(kServiceAccountName))
-		}
-		if gServiceAccountName != "" {
-			errs = errs.Also(validateGCPServiceAccount(gServiceAccountName))
-		}
-		return errs
+	} else if kServiceAccountName != "" {
+		return validateK8sServiceAccount(kServiceAccountName)
 	}
+	return nil
 }
 
 func validateSecret(secret *corev1.SecretKeySelector) *apis.FieldError {
@@ -74,18 +61,6 @@ func validateSecret(secret *corev1.SecretKeySelector) *apis.FieldError {
 	return errs
 }
 
-func validateGCPServiceAccount(gServiceAccountName string) *apis.FieldError {
-	match := gsaValidationRegex.FindStringSubmatch(gServiceAccountName)
-	if len(match) == 0 {
-		return &apis.FieldError{
-			Message: fmt.Sprintf(`invalid value: %s, googleServiceAccount should have format: ^[a-z][a-z0-9-]{5,29}@[a-z][a-z0-9-]{5,29}.iam.gserviceaccount.com$`,
-				gServiceAccountName),
-			Paths: []string{"googleServiceAccount"},
-		}
-	}
-	return nil
-}
-
 func validateK8sServiceAccount(kServiceAccountName string) *apis.FieldError {
 	match := ksaValidationRegex.FindStringSubmatch(kServiceAccountName)
 	if len(match) == 0 {

diff --git a/pkg/apis/duck/v1alpha1/credentials_test.go b/pkg/apis/duck/v1alpha1/credentials_test.go
@@ -80,7 +80,7 @@ func TestValidateCredential(t *testing.T) {
 	defer logtesting.ClearAll()
 
 	for _, tc := range testCases {
-		errs := ValidateCredential(tc.secret, "", tc.serviceAccount)
+		errs := ValidateCredential(tc.secret, tc.serviceAccount)
 		got := errs != nil
 		if diff := cmp.Diff(tc.wantErr, got); diff != "" {
 			t.Errorf("unexpected resource (-want, +got) = %v", diff)

diff --git a/pkg/apis/duck/v1alpha1/identity_types.go b/pkg/apis/duck/v1alpha1/identity_types.go
@@ -19,10 +19,6 @@ import (
 )
 
 type IdentitySpec struct {
-	// GoogleServiceAccount is the GCP service account which has required permissions to poll from a Cloud Pub/Sub subscription.
-	// If not specified, defaults to use secret.
-	// +optional
-	GoogleServiceAccount string `json:"googleServiceAccount,omitempty"`
 	// ServiceAccountName is the k8s service account which binds to a google service account.
 	// This google service account has required permissions to poll from a Cloud Pub/Sub subscription.
 	// If not specified, defaults to use secret.

diff --git a/pkg/apis/events/v1alpha1/cloudauditlogssource_validation.go b/pkg/apis/events/v1alpha1/cloudauditlogssource_validation.go
@@ -51,7 +51,7 @@ func (current *CloudAuditLogsSourceSpec) Validate(ctx context.Context) *apis.Fie
 		errs = errs.Also(apis.ErrMissingField("methodName"))
 	}
 
-	if err := duckv1alpha1.ValidateCredential(current.Secret, current.GoogleServiceAccount, current.ServiceAccountName); err != nil {
+	if err := duckv1alpha1.ValidateCredential(current.Secret, current.ServiceAccountName); err != nil {
 		errs = errs.Also(err)
 	}
 

diff --git a/pkg/apis/events/v1alpha1/cloudbuildsource_validation.go b/pkg/apis/events/v1alpha1/cloudbuildsource_validation.go
@@ -46,7 +46,7 @@ func (current *CloudBuildSourceSpec) Validate(ctx context.Context) *apis.FieldEr
 		errs = errs.Also(err.ViaField("sink"))
 	}
 
-	if err := duckv1alpha1.ValidateCredential(current.Secret, current.GoogleServiceAccount, current.ServiceAccountName); err != nil {
+	if err := duckv1alpha1.ValidateCredential(current.Secret, current.ServiceAccountName); err != nil {
 		errs = errs.Also(err)
 	}
 

diff --git a/pkg/apis/events/v1alpha1/cloudbuildsource_validation_test.go b/pkg/apis/events/v1alpha1/cloudbuildsource_validation_test.go
@@ -286,7 +286,7 @@ func TestCloudBuildSourceCheckImmutableFields(t *testing.T) {
 			updated: CloudBuildSourceSpec{
 				PubSubSpec: duckv1alpha1.PubSubSpec{
 					IdentitySpec: duckv1alpha1.IdentitySpec{
-						GoogleServiceAccount: "new-service-account",
+						ServiceAccountName: "new-service-account",
 					},
 					Secret: &corev1.SecretKeySelector{
 						LocalObjectReference: corev1.LocalObjectReference{

diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go
@@ -100,13 +100,13 @@ func TestCloudPubSubSourceIdentitySpec(t *testing.T) {
 		Spec: CloudPubSubSourceSpec{
 			PubSubSpec: v1alpha1.PubSubSpec{
 				IdentitySpec: v1alpha1.IdentitySpec{
-					GoogleServiceAccount: "test",
+					ServiceAccountName: "test",
 				},
 			},
 		},
 	}
 	want := "test"
-	got := s.IdentitySpec().GoogleServiceAccount
+	got := s.IdentitySpec().ServiceAccountName
 	if diff := cmp.Diff(want, got); diff != "" {
 		t.Errorf("failed to get expected (-want, +got) = %v", diff)
 	}

diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_validation.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_validation.go
@@ -77,7 +77,7 @@ func (current *CloudPubSubSourceSpec) Validate(ctx context.Context) *apis.FieldE
 		}
 	}
 
-	if err := duckv1alpha1.ValidateCredential(current.Secret, current.GoogleServiceAccount, current.ServiceAccountName); err != nil {
+	if err := duckv1alpha1.ValidateCredential(current.Secret, current.ServiceAccountName); err != nil {
 		errs = errs.Also(err)
 	}
 

diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_validation_test.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_validation_test.go
@@ -314,7 +314,7 @@ func TestCloudPubSubSourceCheckImmutableFields(t *testing.T) {
 			updated: CloudPubSubSourceSpec{
 				PubSubSpec: duckv1alpha1.PubSubSpec{
 					IdentitySpec: duckv1alpha1.IdentitySpec{
-						GoogleServiceAccount: "new-service-account",
+						ServiceAccountName: "new-service-account",
 					},
 					Secret: &corev1.SecretKeySelector{
 						LocalObjectReference: corev1.LocalObjectReference{

diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_types_test.go b/pkg/apis/events/v1alpha1/cloudschedulersource_types_test.go
@@ -83,13 +83,13 @@ func TestCloudSchedulerSourceIdentitySpec(t *testing.T) {
 		Spec: CloudSchedulerSourceSpec{
 			PubSubSpec: v1alpha1.PubSubSpec{
 				IdentitySpec: v1alpha1.IdentitySpec{
-					GoogleServiceAccount: "test",
+					ServiceAccountName: "test",
 				},
 			},
 		},
 	}
 	want := "test"
-	got := s.IdentitySpec().GoogleServiceAccount
+	got := s.IdentitySpec().ServiceAccountName
 	if diff := cmp.Diff(want, got); diff != "" {
 		t.Errorf("failed to get expected (-want, +got) = %v", diff)
 	}

diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_validation.go b/pkg/apis/events/v1alpha1/cloudschedulersource_validation.go
@@ -59,7 +59,7 @@ func (current *CloudSchedulerSourceSpec) Validate(ctx context.Context) *apis.Fie
 		errs = errs.Also(apis.ErrMissingField("data"))
 	}
 
-	if err := duckv1alpha1.ValidateCredential(current.Secret, current.GoogleServiceAccount, current.ServiceAccountName); err != nil {
+	if err := duckv1alpha1.ValidateCredential(current.Secret, current.ServiceAccountName); err != nil {
 		errs = errs.Also(err)
 	}
 

diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_validation_test.go b/pkg/apis/events/v1alpha1/cloudschedulersource_validation_test.go
@@ -442,7 +442,7 @@ func TestCloudSchedulerSourceSpecCheckImmutableFields(t *testing.T) {
 				Data:     schedulerWithSecret.Data,
 				PubSubSpec: duckv1alpha1.PubSubSpec{
 					IdentitySpec: duckv1alpha1.IdentitySpec{
-						GoogleServiceAccount: "new-service-account",
+						ServiceAccountName: "new-service-account",
 					},
 					SourceSpec: schedulerWithSecret.SourceSpec,
 					Secret:     schedulerWithSecret.Secret,

diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_types_test.go b/pkg/apis/events/v1alpha1/cloudstoragesource_types_test.go
@@ -81,13 +81,13 @@ func TestCloudStorageSourceIdentitySpec(t *testing.T) {
 		Spec: CloudStorageSourceSpec{
 			PubSubSpec: v1alpha1.PubSubSpec{
 				IdentitySpec: v1alpha1.IdentitySpec{
-					GoogleServiceAccount: "test",
+					ServiceAccountName: "test",
 				},
 			},
 		},
 	}
 	want := "test"
-	got := s.IdentitySpec().GoogleServiceAccount
+	got := s.IdentitySpec().ServiceAccountName
 	if diff := cmp.Diff(want, got); diff != "" {
 		t.Errorf("failed to get expected (-want, +got) = %v", diff)
 	}

diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_validation.go b/pkg/apis/events/v1alpha1/cloudstoragesource_validation.go
@@ -49,7 +49,7 @@ func (current *CloudStorageSourceSpec) Validate(ctx context.Context) *apis.Field
 		errs = errs.Also(apis.ErrMissingField("bucket"))
 	}
 
-	if err := duckv1alpha1.ValidateCredential(current.Secret, current.GoogleServiceAccount, current.ServiceAccountName); err != nil {
+	if err := duckv1alpha1.ValidateCredential(current.Secret, current.ServiceAccountName); err != nil {
 		errs = errs.Also(err)
 	}
 

diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_validation_test.go b/pkg/apis/events/v1alpha1/cloudstoragesource_validation_test.go
@@ -454,7 +454,7 @@ func TestCheckImmutableFields(t *testing.T) {
 				PayloadFormat:    storageSourceSpec.PayloadFormat,
 				PubSubSpec: duckv1alpha1.PubSubSpec{
 					IdentitySpec: duckv1alpha1.IdentitySpec{
-						GoogleServiceAccount: "new-service-account",
+						ServiceAccountName: "new-service-account",
 					},
 					SourceSpec: storageSourceSpec.SourceSpec,
 					Secret:     storageSourceSpec.Secret,

diff --git a/pkg/apis/messaging/v1alpha1/channel_types_test.go b/pkg/apis/messaging/v1alpha1/channel_types_test.go
@@ -47,12 +47,12 @@ func TestChannelIdentitySpec(t *testing.T) {
 	s := &Channel{
 		Spec: ChannelSpec{
 			IdentitySpec: duckv1alpha1.IdentitySpec{
-				GoogleServiceAccount: "test",
+				ServiceAccountName: "test",
 			},
 		},
 	}
 	want := "test"
-	got := s.IdentitySpec().GoogleServiceAccount
+	got := s.IdentitySpec().ServiceAccountName
 	if diff := cmp.Diff(want, got); diff != "" {
 		t.Errorf("failed to get expected (-want, +got) = %v", diff)
 	}

diff --git a/pkg/apis/messaging/v1alpha1/channel_validation.go b/pkg/apis/messaging/v1alpha1/channel_validation.go
@@ -44,7 +44,7 @@ func (cs *ChannelSpec) Validate(ctx context.Context) *apis.FieldError {
 		}
 	}
 
-	if err := duckv1alpha1.ValidateCredential(cs.Secret, cs.GoogleServiceAccount, cs.ServiceAccountName); err != nil {
+	if err := duckv1alpha1.ValidateCredential(cs.Secret, cs.ServiceAccountName); err != nil {
 		errs = errs.Also(err)
 	}
 
@@ -69,7 +69,7 @@ func (current *Channel) CheckImmutableFields(ctx context.Context, original *Chan
 		}
 	}
 
-	if diff := cmp.Diff(original.Spec.GoogleServiceAccount, current.Spec.GoogleServiceAccount); diff != "" {
+	if diff := cmp.Diff(original.Spec.ServiceAccountName, current.Spec.ServiceAccountName); diff != "" {
 		errs = errs.Also(&apis.FieldError{
 			Message: "Immutable fields changed (-old +new)",
 			Paths:   []string{"status", "serviceAccount"},

diff --git a/pkg/apis/messaging/v1alpha1/channel_validation_test.go b/pkg/apis/messaging/v1alpha1/channel_validation_test.go
@@ -242,7 +242,7 @@ func TestCheckImmutableFields(t *testing.T) {
 			orig: &channelSpec,
 			updated: ChannelSpec{
 				IdentitySpec: duckv1alpha1.IdentitySpec{
-					GoogleServiceAccount: "new-service-account",
+					ServiceAccountName: "new-service-account",
 				},
 			},
 			allowed: false,

diff --git a/pkg/reconciler/events/auditlogs/auditlogs.go b/pkg/reconciler/events/auditlogs/auditlogs.go
@@ -72,8 +72,8 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, s *v1alpha1.CloudAuditLo
 	s.Status.InitializeConditions()
 	s.Status.ObservedGeneration = s.Generation
 
-	// If GCP ServiceAccount is provided, reconcile workload identity.
-	if s.Spec.ServiceAccountName != "" || s.Spec.GoogleServiceAccount != "" {
+	// If ServiceAccountName is provided, reconcile workload identity.
+	if s.Spec.ServiceAccountName != "" {
 		if _, err := c.Identity.ReconcileWorkloadIdentity(ctx, s.Spec.Project, s); err != nil {
 			return reconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile CloudAuditLogsSource workload identity: %s", err.Error())
 		}
@@ -185,9 +185,10 @@ func (c *Reconciler) deleteSink(ctx context.Context, s *v1alpha1.CloudAuditLogsS
 }
 
 func (c *Reconciler) FinalizeKind(ctx context.Context, s *v1alpha1.CloudAuditLogsSource) reconciler.Event {
-	// If k8s ServiceAccount exists and it only has one ownerReference, remove the corresponding GCP ServiceAccount iam policy binding.
+	// If k8s ServiceAccount exists, binds to the default GCP ServiceAccount, and it only has one ownerReference,
+	// remove the corresponding GCP ServiceAccount iam policy binding.
 	// No need to delete k8s ServiceAccount, it will be automatically handled by k8s Garbage Collection.
-	if s.Spec.ServiceAccountName != "" || s.Spec.GoogleServiceAccount != "" {
+	if s.Spec.ServiceAccountName != "" {
 		if err := c.Identity.DeleteWorkloadIdentity(ctx, s.Spec.Project, s); err != nil {
 			return reconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete CloudAuditLogsSource workload identity: %s", err.Error())
 		}