From a12cfb0e24ef785a5125a0284682d6018c89b0b7 Mon Sep 17 00:00:00 2001 From: Grace Gao <52978759+grac3gao@users.noreply.github.com> Date: Tue, 9 Jun 2020 10:44:33 -0700 Subject: [PATCH] Remove spec.googleServiceAccount in v1alpha1 (#1249) * remove googleServiceAccount * comments --- config/core/resources/channel.yaml | 6 - .../core/resources/cloudauditlogssource.yaml | 6 - config/core/resources/cloudbuildsource.yaml | 6 - config/core/resources/cloudpubsubsource.yaml | 6 - .../core/resources/cloudschedulersource.yaml | 6 - config/core/resources/cloudstoragesource.yaml | 6 - config/core/resources/pullsubscription.yaml | 3 - config/core/resources/topic.yaml | 3 - pkg/apis/duck/v1alpha1/credentials.go | 33 +-- pkg/apis/duck/v1alpha1/credentials_test.go | 2 +- pkg/apis/duck/v1alpha1/identity_types.go | 4 - .../cloudauditlogssource_validation.go | 2 +- .../v1alpha1/cloudbuildsource_validation.go | 2 +- .../cloudbuildsource_validation_test.go | 2 +- .../v1alpha1/cloudpubsubsource_types_test.go | 4 +- .../v1alpha1/cloudpubsubsource_validation.go | 2 +- .../cloudpubsubsource_validation_test.go | 2 +- .../cloudschedulersource_types_test.go | 4 +- .../cloudschedulersource_validation.go | 2 +- .../cloudschedulersource_validation_test.go | 2 +- .../v1alpha1/cloudstoragesource_types_test.go | 4 +- .../v1alpha1/cloudstoragesource_validation.go | 2 +- .../cloudstoragesource_validation_test.go | 2 +- .../messaging/v1alpha1/channel_types_test.go | 4 +- .../messaging/v1alpha1/channel_validation.go | 4 +- .../v1alpha1/channel_validation_test.go | 2 +- pkg/reconciler/events/auditlogs/auditlogs.go | 9 +- .../events/auditlogs/auditlogs_test.go | 37 --- pkg/reconciler/events/build/build.go | 9 +- pkg/reconciler/events/build/build_test.go | 34 --- pkg/reconciler/events/pubsub/pubsub.go | 9 +- pkg/reconciler/events/pubsub/pubsub_test.go | 34 --- pkg/reconciler/events/scheduler/scheduler.go | 9 +- .../events/scheduler/scheduler_test.go | 42 ---- pkg/reconciler/events/storage/storage.go | 9 +- pkg/reconciler/events/storage/storage_test.go | 41 --- pkg/reconciler/identity/reconciler.go | 17 -- pkg/reconciler/identity/reconciler_test.go | 236 +----------------- .../identity/resources/service_account.go | 9 - .../resources/service_account_test.go | 13 +- .../intevents/pullsubscription/reconciler.go | 10 +- .../resources/receive_adapter.go | 13 - .../resources/receive_adapter_test.go | 14 +- .../intevents/resources/pullsubscription.go | 3 +- pkg/reconciler/intevents/resources/topic.go | 3 +- .../intevents/topic/resources/publisher.go | 13 - .../topic/resources/publisher_test.go | 8 +- pkg/reconciler/intevents/topic/topic.go | 10 +- pkg/reconciler/messaging/channel/channel.go | 12 +- .../messaging/channel/channel_test.go | 39 --- .../channel/resources/pullsubscription.go | 3 +- .../messaging/channel/resources/topic.go | 3 +- pkg/reconciler/testing/auditlogs.go | 6 - pkg/reconciler/testing/build.go | 6 - pkg/reconciler/testing/channel.go | 6 - pkg/reconciler/testing/pubsub.go | 6 - pkg/reconciler/testing/scheduler.go | 6 - pkg/reconciler/testing/storage.go | 6 - 58 files changed, 89 insertions(+), 707 deletions(-) diff --git a/config/core/resources/channel.yaml b/config/core/resources/channel.yaml index 02b166d5cd..ba3d48133d 100644 --- 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 index 4de46a466e..2ae2e58fe2 100644 --- 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 index 3bb4a81ca1..ea96e692b1 100644 --- 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 index 578a95f9be..fadb57c14b 100644 --- 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 index a6bad81cc3..e958c2aeea 100644 --- 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 index 4d67763591..73185b8118 100644 --- 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 index b67ca2dc14..57bbd497b4 100644 --- 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 index 1b36cfd3c2..19ae9eeca0 100644 --- 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 index f78c467266..c9a62a22e1 100644 --- a/pkg/apis/duck/v1alpha1/credentials.go +++ b/pkg/apis/duck/v1alpha1/credentials.go @@ -30,20 +30,13 @@ 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", @@ -51,16 +44,10 @@ func ValidateCredential(secret *corev1.SecretKeySelector, gServiceAccountName st } } 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 index 926182c05c..16eb468f0e 100644 --- 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 index 4e3d2f653e..48568672e1 100644 --- 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 index 3f9018ba54..2d49920db5 100644 --- 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 index f579b73d24..da2bdad815 100644 --- 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 index 840bd325a4..11cfa80306 100644 --- 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 index 7988779cbb..94113e9825 100644 --- 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 index ce8beee20f..7393360744 100644 --- 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 index 3992b4d558..296122f90a 100644 --- 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 index fe0df1b2ed..d3358fbdca 100644 --- 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 index 15d1a4e05f..1f70d0d5a2 100644 --- 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 index 1d4bf02288..9600323049 100644 --- 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 index cb3bf8850a..47aeaad60e 100644 --- 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 index ee1dd8578d..383fc3ba88 100644 --- 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 index 99bf789a1e..af129da7d6 100644 --- 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 index 94ecf39418..b9508d65e3 100644 --- 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 index 89a02ee9ce..6584717aa5 100644 --- 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 index 5fce838f23..0796e12757 100644 --- 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 index c5e3c511c4..2a5c4a0368 100644 --- 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()) } diff --git a/pkg/reconciler/events/auditlogs/auditlogs_test.go b/pkg/reconciler/events/auditlogs/auditlogs_test.go index dd563f1fcc..1d1ad5217b 100644 --- a/pkg/reconciler/events/auditlogs/auditlogs_test.go +++ b/pkg/reconciler/events/auditlogs/auditlogs_test.go @@ -1326,43 +1326,6 @@ func TestAllCases(t *testing.T) { Name: sourceName, }, }, - }, { - Name: "delete failed with getting k8s service account error", - Objects: []runtime.Object{ - NewCloudAuditLogsSource(sourceName, testNS, - WithCloudAuditLogsSourceUID(sourceUID), - WithCloudAuditLogsSourceSink(sinkGVK, sinkName), - WithCloudAuditLogsSourceMethodName(testMethodName), - WithCloudAuditLogsSourceServiceName(testServiceName), - WithInitCloudAuditLogsSourceConditions, - WithCloudAuditLogsSourceGCPServiceAccount(gServiceAccount), - WithCloudAuditLogsSourceDeletionTimestamp, - WithCloudAuditLogsSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudAuditLogsSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - ), - }, - Key: testNS + "/" + sourceName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudAuditLogsSource(sourceName, testNS, - WithCloudAuditLogsSourceUID(sourceUID), - WithCloudAuditLogsSourceMethodName(testMethodName), - WithCloudAuditLogsSourceServiceName(testServiceName), - WithCloudAuditLogsSourceSink(sinkGVK, sinkName), - WithInitCloudAuditLogsSourceConditions, - WithCloudAuditLogsSourceGCPServiceAccount(gServiceAccount), - WithCloudAuditLogsSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123-fake-cluster-name" not found`), - WithCloudAuditLogsSourceDeletionTimestamp, - WithCloudAuditLogsSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudAuditLogsSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - ), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudAuditLogsSource workload identity: getting k8s service account failed with: serviceaccounts "test123-fake-cluster-name" not found`), - }, }} defer logtesting.ClearAll() diff --git a/pkg/reconciler/events/build/build.go b/pkg/reconciler/events/build/build.go index f57e69af81..484dba3c58 100644 --- a/pkg/reconciler/events/build/build.go +++ b/pkg/reconciler/events/build/build.go @@ -65,8 +65,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, build *v1alpha1.CloudBui build.Status.InitializeConditions() build.Status.ObservedGeneration = build.Generation - // If GCP ServiceAccount is provided, reconcile workload identity. - if build.Spec.ServiceAccountName != "" || build.Spec.GoogleServiceAccount != "" { + // If ServiceAccountName is provided, reconcile workload identity. + if build.Spec.ServiceAccountName != "" { if _, err := r.Identity.ReconcileWorkloadIdentity(ctx, build.Spec.Project, build); err != nil { return pkgreconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile CloudBuildSource workload identity: %s", err.Error()) } @@ -80,9 +80,10 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, build *v1alpha1.CloudBui } func (r *Reconciler) FinalizeKind(ctx context.Context, build *v1alpha1.CloudBuildSource) pkgreconciler.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 build.Spec.ServiceAccountName != "" || build.Spec.GoogleServiceAccount != "" { + if build.Spec.ServiceAccountName != "" { if err := r.Identity.DeleteWorkloadIdentity(ctx, build.Spec.Project, build); err != nil { return pkgreconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete CloudBuildSource workload identity: %s", err.Error()) } diff --git a/pkg/reconciler/events/build/build_test.go b/pkg/reconciler/events/build/build_test.go index ae0b2e0890..3ea359ed1b 100644 --- a/pkg/reconciler/events/build/build_test.go +++ b/pkg/reconciler/events/build/build_test.go @@ -361,40 +361,6 @@ func TestAllCases(t *testing.T) { Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", buildName), Eventf(corev1.EventTypeNormal, reconciledSuccessReason, `CloudBuildSource reconciled: "%s/%s"`, testNS, buildName), }, - }, { - Name: "pullsubscription deleted with getting k8s service account error", - Objects: []runtime.Object{ - NewCloudBuildSource(buildName, testNS, - WithCloudBuildSourceObjectMetaGeneration(generation), - WithCloudBuildSourceTopic(testTopicID), - WithCloudBuildSourceSink(sinkGVK, sinkName), - WithCloudBuildSourceDeletionTimestamp, - WithCloudBuildSourceGCPServiceAccount(gServiceAccount), - WithCloudBuildSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudBuildSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - ), - newSink(), - }, - Key: testNS + "/" + buildName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudBuildSource(buildName, testNS, - WithCloudBuildSourceObjectMetaGeneration(generation), - WithCloudBuildSourceTopic(testTopicID), - WithCloudBuildSourceSink(sinkGVK, sinkName), - WithCloudBuildSourceDeletionTimestamp, - WithCloudBuildSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123-fake-cluster-name" not found`), - WithCloudBuildSourceGCPServiceAccount(gServiceAccount), - WithCloudBuildSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudBuildSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - ), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudBuildSource workload identity: getting k8s service account failed with: serviceaccounts "test123-fake-cluster-name" not found`), - }, }} defer logtesting.ClearAll() diff --git a/pkg/reconciler/events/pubsub/pubsub.go b/pkg/reconciler/events/pubsub/pubsub.go index fe567e240a..4539d688ab 100644 --- a/pkg/reconciler/events/pubsub/pubsub.go +++ b/pkg/reconciler/events/pubsub/pubsub.go @@ -57,8 +57,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, pubsub *v1alpha1.CloudPu pubsub.Status.InitializeConditions() pubsub.Status.ObservedGeneration = pubsub.Generation - // If GCP ServiceAccount is provided, reconcile workload identity. - if pubsub.Spec.ServiceAccountName != "" || pubsub.Spec.GoogleServiceAccount != "" { + // If ServiceAccountName is provided, reconcile workload identity. + if pubsub.Spec.ServiceAccountName != "" { if _, err := r.Identity.ReconcileWorkloadIdentity(ctx, pubsub.Spec.Project, pubsub); err != nil { return pkgreconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile CloudPubSubSource workload identity: %s", err.Error()) } @@ -72,9 +72,10 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, pubsub *v1alpha1.CloudPu } func (r *Reconciler) FinalizeKind(ctx context.Context, pubsub *v1alpha1.CloudPubSubSource) pkgreconciler.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 pubsub.Spec.ServiceAccountName != "" || pubsub.Spec.GoogleServiceAccount != "" { + if pubsub.Spec.ServiceAccountName != "" { if err := r.Identity.DeleteWorkloadIdentity(ctx, pubsub.Spec.Project, pubsub); err != nil { return pkgreconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete CloudPubSubSource workload identity: %s", err.Error()) } diff --git a/pkg/reconciler/events/pubsub/pubsub_test.go b/pkg/reconciler/events/pubsub/pubsub_test.go index ced0231a61..da95138bab 100644 --- a/pkg/reconciler/events/pubsub/pubsub_test.go +++ b/pkg/reconciler/events/pubsub/pubsub_test.go @@ -360,40 +360,6 @@ func TestAllCases(t *testing.T) { Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", pubsubName), Eventf(corev1.EventTypeNormal, reconciledSuccessReason, `CloudPubSubSource reconciled: "%s/%s"`, testNS, pubsubName), }, - }, { - Name: "pullsubscription deleted with getting k8s service account error", - Objects: []runtime.Object{ - NewCloudPubSubSource(pubsubName, testNS, - WithCloudPubSubSourceObjectMetaGeneration(generation), - WithCloudPubSubSourceTopic(testTopicID), - WithCloudPubSubSourceSink(sinkGVK, sinkName), - WithCloudPubSubSourceDeletionTimestamp, - WithCloudPubSubSourceGCPServiceAccount(gServiceAccount), - WithCloudPubSubSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudPubSubSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - ), - newSink(), - }, - Key: testNS + "/" + pubsubName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudPubSubSource(pubsubName, testNS, - WithCloudPubSubSourceObjectMetaGeneration(generation), - WithCloudPubSubSourceTopic(testTopicID), - WithCloudPubSubSourceSink(sinkGVK, sinkName), - WithCloudPubSubSourceDeletionTimestamp, - WithCloudPubSubSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123-fake-cluster-name" not found`), - WithCloudPubSubSourceGCPServiceAccount(gServiceAccount), - WithCloudPubSubSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudPubSubSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - ), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudPubSubSource workload identity: getting k8s service account failed with: serviceaccounts "test123-fake-cluster-name" not found`), - }, }} defer logtesting.ClearAll() diff --git a/pkg/reconciler/events/scheduler/scheduler.go b/pkg/reconciler/events/scheduler/scheduler.go index 416d0f835a..aeff2eda90 100644 --- a/pkg/reconciler/events/scheduler/scheduler.go +++ b/pkg/reconciler/events/scheduler/scheduler.go @@ -72,8 +72,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, scheduler *v1alpha1.Clou scheduler.Status.InitializeConditions() scheduler.Status.ObservedGeneration = scheduler.Generation - // If GCP ServiceAccount is provided, reconcile workload identity. - if scheduler.Spec.ServiceAccountName != "" || scheduler.Spec.GoogleServiceAccount != "" { + // If ServiceAccountName is provided, reconcile workload identity. + if scheduler.Spec.ServiceAccountName != "" { if _, err := r.Identity.ReconcileWorkloadIdentity(ctx, scheduler.Spec.Project, scheduler); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile CloudSchedulerSource workload identity: %s", err.Error()) } @@ -185,9 +185,10 @@ func (r *Reconciler) deleteJob(ctx context.Context, scheduler *v1alpha1.CloudSch } func (r *Reconciler) FinalizeKind(ctx context.Context, scheduler *v1alpha1.CloudSchedulerSource) 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 scheduler.Spec.ServiceAccountName != "" || scheduler.Spec.GoogleServiceAccount != "" { + if scheduler.Spec.ServiceAccountName != "" { if err := r.Identity.DeleteWorkloadIdentity(ctx, scheduler.Spec.Project, scheduler); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete CloudSchedulerSource workload identity: %s", err.Error()) } diff --git a/pkg/reconciler/events/scheduler/scheduler_test.go b/pkg/reconciler/events/scheduler/scheduler_test.go index 6d75e8944a..992533744a 100644 --- a/pkg/reconciler/events/scheduler/scheduler_test.go +++ b/pkg/reconciler/events/scheduler/scheduler_test.go @@ -1177,48 +1177,6 @@ func TestAllCases(t *testing.T) { DeleteJobErr: gstatus.Error(codes.NotFound, "delete-job-induced-error"), }, }, - }, { - Name: "scheduler deleted with getting k8s service account error", - Objects: []runtime.Object{ - NewCloudSchedulerSource(schedulerName, testNS, - WithCloudSchedulerSourceProject(testProject), - WithCloudSchedulerSourceSink(sinkGVK, sinkName), - WithCloudSchedulerSourceLocation(location), - WithCloudSchedulerSourceData(testData), - WithCloudSchedulerSourceSchedule(onceAMinuteSchedule), - WithInitCloudSchedulerSourceConditions, - WithCloudSchedulerSourceSinkURI(schedulerSinkURL), - WithCloudSchedulerSourceDeletionTimestamp, - WithCloudSchedulerSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - WithCloudSchedulerSourceGCPServiceAccount(gServiceAccount), - WithCloudSchedulerSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - ), - newSink(), - }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudSchedulerSource(schedulerName, testNS, - WithCloudSchedulerSourceProject(testProject), - WithCloudSchedulerSourceSink(sinkGVK, sinkName), - WithCloudSchedulerSourceLocation(location), - WithCloudSchedulerSourceData(testData), - WithCloudSchedulerSourceSchedule(onceAMinuteSchedule), - WithInitCloudSchedulerSourceConditions, - WithCloudSchedulerSourceSinkURI(schedulerSinkURL), - WithCloudSchedulerSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - WithCloudSchedulerSourceDeletionTimestamp, - WithCloudSchedulerSourceGCPServiceAccount(gServiceAccount), - WithCloudSchedulerSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudSchedulerSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123-fake-cluster-name" not found`), - ), - }}, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudSchedulerSource workload identity: getting k8s service account failed with: serviceaccounts "test123-fake-cluster-name" not found`), - }, }} defer logtesting.ClearAll() diff --git a/pkg/reconciler/events/storage/storage.go b/pkg/reconciler/events/storage/storage.go index c54350db73..59d0853623 100644 --- a/pkg/reconciler/events/storage/storage.go +++ b/pkg/reconciler/events/storage/storage.go @@ -86,8 +86,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, storage *v1alpha1.CloudS storage.Status.InitializeConditions() storage.Status.ObservedGeneration = storage.Generation - // If GCP ServiceAccount is provided, reconcile workload identity. - if storage.Spec.ServiceAccountName != "" || storage.Spec.GoogleServiceAccount != "" { + // If ServiceAccountName is provided, reconcile workload identity. + if storage.Spec.ServiceAccountName != "" { if _, err := r.Identity.ReconcileWorkloadIdentity(ctx, storage.Spec.Project, storage); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile CloudStorageSource workload identity: %s", err.Error()) } @@ -241,9 +241,10 @@ func (r *Reconciler) deleteNotification(ctx context.Context, storage *v1alpha1.C } func (r *Reconciler) FinalizeKind(ctx context.Context, storage *v1alpha1.CloudStorageSource) 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 storage.Spec.ServiceAccountName != "" || storage.Spec.GoogleServiceAccount != "" { + if storage.Spec.ServiceAccountName != "" { if err := r.Identity.DeleteWorkloadIdentity(ctx, storage.Spec.Project, storage); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete CloudStorageSource workload identity: %s", err.Error()) } diff --git a/pkg/reconciler/events/storage/storage_test.go b/pkg/reconciler/events/storage/storage_test.go index 1cbad21a08..00f5d0fd33 100644 --- a/pkg/reconciler/events/storage/storage_test.go +++ b/pkg/reconciler/events/storage/storage_test.go @@ -1087,47 +1087,6 @@ func TestAllCases(t *testing.T) { }, WantStatusUpdates: nil, }, { - Name: "delete fails with getting k8s service account error", - Objects: []runtime.Object{ - NewCloudStorageSource(storageName, testNS, - WithCloudStorageSourceProject(testProject), - WithCloudStorageSourceObjectMetaGeneration(generation), - WithCloudStorageSourceBucket(bucket), - WithCloudStorageSourceSink(sinkGVK, sinkName), - WithCloudStorageSourceSinkURI(storageSinkURL), - WithCloudStorageSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - WithCloudStorageSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudStorageSourceGCPServiceAccount(gServiceAccount), - WithDeletionTimestamp(), - ), - newSink(), - }, - Key: testNS + "/" + storageName, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", - `Failed to delete CloudStorageSource workload identity: getting k8s service account failed with: serviceaccounts "test123-fake-cluster-name" not found`), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudStorageSource(storageName, testNS, - WithCloudStorageSourceProject(testProject), - WithCloudStorageSourceObjectMetaGeneration(generation), - WithCloudStorageSourceBucket(bucket), - WithCloudStorageSourceSink(sinkGVK, sinkName), - WithCloudStorageSourceSinkURI(storageSinkURL), - WithCloudStorageSourceGCPServiceAccount(gServiceAccount), - WithCloudStorageSourceServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithCloudStorageSourceAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }), - WithCloudStorageSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", - `serviceaccounts "test123-fake-cluster-name" not found`), - WithDeletionTimestamp(), - ), - }}, - }, - { Name: "successfully deleted storage when bucket doesn't exist", Objects: []runtime.Object{ NewCloudStorageSource(storageName, testNS, diff --git a/pkg/reconciler/identity/reconciler.go b/pkg/reconciler/identity/reconciler.go index 0878e32658..61222c3e9d 100644 --- a/pkg/reconciler/identity/reconciler.go +++ b/pkg/reconciler/identity/reconciler.go @@ -33,7 +33,6 @@ import ( "knative.dev/pkg/ptr" "github.com/google/knative-gcp/pkg/apis/configs/gcpauth" - "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" duck "github.com/google/knative-gcp/pkg/duck/v1alpha1" metadataClient "github.com/google/knative-gcp/pkg/gclient/metadata" "github.com/google/knative-gcp/pkg/reconciler/identity/iam" @@ -153,21 +152,6 @@ func (i *Identity) DeleteWorkloadIdentity(ctx context.Context, projectID string, // getGoogleServiceAccountName will return Google service account name and corresponding raw Kubernetes service account name. func (i *Identity) getGoogleServiceAccountName(ctx context.Context, identifiable duck.Identifiable) (resources.IdentityNames, error) { namespace := identifiable.GetObjectMeta().GetNamespace() - clusterName := identifiable.GetObjectMeta().GetAnnotations()[v1alpha1.ClusterNameAnnotation] - if clusterName == "" { - err := fmt.Errorf(`unable to get cluster name, please provide it by adding annotation "%s=$CLUSTER_NAME" to source`, v1alpha1.ClusterNameAnnotation) - return resources.IdentityNames{}, fmt.Errorf("failed to get cluster name: %w", err) - } - - if identifiable.IdentitySpec().GoogleServiceAccount != "" { - googleServiceAccount := identifiable.IdentitySpec().GoogleServiceAccount - return resources.IdentityNames{ - KServiceAccountName: resources.GenerateServiceAccountName(googleServiceAccount, clusterName), - GoogleServiceAccountName: googleServiceAccount, - Namespace: namespace, - ClusterName: clusterName, - }, nil - } ad := i.gcpAuthStore.Load() if ad == nil || ad.GCPAuthDefaults == nil { logging.FromContext(ctx).Desugar().Error("Failed to get default config from GCP auth configmap") @@ -177,7 +161,6 @@ func (i *Identity) getGoogleServiceAccountName(ctx context.Context, identifiable KServiceAccountName: identifiable.IdentitySpec().ServiceAccountName, GoogleServiceAccountName: ad.GCPAuthDefaults.WorkloadIdentityGSA(namespace, identifiable.IdentitySpec().ServiceAccountName), Namespace: namespace, - ClusterName: clusterName, }, nil } diff --git a/pkg/reconciler/identity/reconciler_test.go b/pkg/reconciler/identity/reconciler_test.go index 34e5177af0..bd81cbe0b9 100644 --- a/pkg/reconciler/identity/reconciler_test.go +++ b/pkg/reconciler/identity/reconciler_test.go @@ -22,9 +22,6 @@ import ( "strings" "testing" - logtesting "knative.dev/pkg/logging/testing" - - "github.com/google/knative-gcp/pkg/apis/configs/gcpauth" gclient "github.com/google/knative-gcp/pkg/gclient/iam/admin" testingMetadataClient "github.com/google/knative-gcp/pkg/gclient/metadata/testing" @@ -190,11 +187,15 @@ func TestKSADeletes(t *testing.T) { name string wantDeletes []clientgotesting.DeleteActionImpl objects []runtime.Object + config *corev1.ConfigMap wantErrCode codes.Code }{ // Due to the limitation mentioned in https://github.com/google/knative-gcp/issues/1037, // skip test case "delete k8s service account, failed to get cluster name annotation." { + name: "non-default serviceAccountName, no need to run finalizer", + config: ConfigMapFromTestFile(t, "config-gcp-auth-empty", "default-auth-config"), + }, { name: "default serviceAccountName, delete k8s service account, failed with removing iam policy binding.", objects: []runtime.Object{ NewServiceAccount(kServiceAccountName, testNS, gServiceAccountName, @@ -208,6 +209,7 @@ func TestKSADeletes(t *testing.T) { }}), ), }, + config: ConfigMapFromTestFile(t, "config-gcp-auth", "default-auth-config"), wantErrCode: codes.NotFound, }, { name: "default serviceAccountName, no need to remove k8s service account", @@ -230,6 +232,7 @@ func TestKSADeletes(t *testing.T) { }}), ), }, + config: ConfigMapFromTestFile(t, "config-gcp-auth", "default-auth-config"), }} for _, tc := range testCases { @@ -248,232 +251,11 @@ func TestKSADeletes(t *testing.T) { identity := &Identity{ kubeClient: cs, policyManager: m, - gcpAuthStore: gcpauth.NewStore(logtesting.TestLogger(t)), - } - identifiable := NewCloudPubSubSource(identifiableName, testNS, - WithCloudPubSubSourceGCPServiceAccount(gServiceAccountName), - WithCloudPubSubSourceServiceAccountName("test")) - identifiable.Spec.ServiceAccountName = "test" - identifiable.SetAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }) - - arl := pkgtesting.ActionRecorderList{cs} - err = identity.DeleteWorkloadIdentity(ctx, projectID, identifiable) - - var statusErr interface{ GRPCStatus() *status.Status } - if errors.As(err, &statusErr) { - if code := statusErr.GRPCStatus().Code(); code != tc.wantErrCode { - t.Fatalf("error code: want %v, got %v", tc.wantErrCode, code) - } - } else { - if tc.wantErrCode != codes.OK { - t.Fatal(err) - } - } - - // validate deletes - actions, err := arl.ActionsByVerb() - if err != nil { - t.Errorf("Error capturing actions by verb: %q", err) - } - for i, want := range tc.wantDeletes { - if i >= len(actions.Deletes) { - t.Errorf("Missing delete: %#v", want) - continue - } - got := actions.Deletes[i] - if got.GetName() != want.GetName() { - t.Errorf("Unexpected delete[%d]: %#v", i, got) - } - if got.GetResource() != want.GetResource() { - t.Errorf("Unexpected delete[%d]: %#v wanted: %#v", i, got, want) - } - } - if got, want := len(actions.Deletes), len(tc.wantDeletes); got > want { - for _, extra := range actions.Deletes[want:] { - t.Errorf("Extra delete: %s/%s", extra.GetNamespace(), extra.GetName()) - } - } - }) - } -} - -func TestCreates(t *testing.T) { - t.Parallel() - testCases := []struct { - name string - objects []runtime.Object - expectedServiceAccount *corev1.ServiceAccount - wantCreates []runtime.Object - wantErrCode codes.Code - }{ - // Due to the limitation mentioned in https://github.com/google/knative-gcp/issues/1037, - // skip test case "k8s service account doesn't exist, failed to get cluster name annotation." - { - name: "k8s service account doesn't exist, create it", - wantCreates: []runtime.Object{ - NewServiceAccount(kServiceAccountName, testNS, gServiceAccountName), - }, - expectedServiceAccount: NewServiceAccount(kServiceAccountName, testNS, gServiceAccountName, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudPubSubSource", - UID: "test-pubsub-uid", - Name: identifiableName, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - wantErrCode: codes.NotFound, - }, { - name: "k8s service account exists, but doesn't have ownerReference", - objects: []runtime.Object{ - NewServiceAccount(kServiceAccountName, testNS, gServiceAccountName), - }, - expectedServiceAccount: NewServiceAccount(kServiceAccountName, testNS, gServiceAccountName, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudPubSubSource", - UID: "test-pubsub-uid", - Name: identifiableName, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - wantErrCode: codes.NotFound, - }} - - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - cs := fakeKubeClient.NewSimpleClientset(tc.objects...) - iamClient := gclient.NewTestClient() - m, err := iam.NewIAMPolicyManager(ctx, iamClient) - if err != nil { - t.Fatal(err) - } - identity := &Identity{ - kubeClient: cs, - policyManager: m, - gcpAuthStore: gcpauth.NewStore(logtesting.TestLogger(t)), - } - identifiable := NewCloudPubSubSource(identifiableName, testNS, - WithCloudPubSubSourceGCPServiceAccount(gServiceAccountName)) - identifiable.SetAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, - }) - - arl := pkgtesting.ActionRecorderList{cs} - kserviceAccount, err := identity.ReconcileWorkloadIdentity(ctx, projectID, identifiable) - - var statusErr interface{ GRPCStatus() *status.Status } - if errors.As(err, &statusErr) { - if code := statusErr.GRPCStatus().Code(); code != tc.wantErrCode { - t.Fatalf("error code: want %v, got %v", tc.wantErrCode, code) - } - } else { - if tc.wantErrCode != codes.OK { - t.Fatal(err) - } - } - if diff := cmp.Diff(tc.expectedServiceAccount, kserviceAccount, ignoreLastTransitionTime); diff != "" { - t.Errorf("unexpected kserviceAccount (-want, +got) = %v", diff) - } - - // Validate creates. - actions, err := arl.ActionsByVerb() - if err != nil { - t.Fatal(err) - } - for i, want := range tc.wantCreates { - if i >= len(actions.Creates) { - t.Errorf("Missing create: %#v", want) - continue - } - got := actions.Creates[i] - obj := got.GetObject() - if diff := cmp.Diff(want, obj); diff != "" { - t.Errorf("Unexpected create (-want, +got): %s", diff) - } - } - }) - } -} - -func TestDeletes(t *testing.T) { - t.Parallel() - testCases := []struct { - name string - wantDeletes []clientgotesting.DeleteActionImpl - objects []runtime.Object - wantErrCode codes.Code - }{ - // Due to the limitation mentioned in https://github.com/google/knative-gcp/issues/1037, - // skip test case "delete k8s service account, failed to get cluster name annotation." - { - name: "delete k8s service account, failed with removing iam policy binding.", - objects: []runtime.Object{ - NewServiceAccount(kServiceAccountName, testNS, gServiceAccountName, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudPubSubSource", - UID: "test-pubsub-uid", - Name: identifiableName, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - }, - wantErrCode: codes.NotFound, - }, { - name: "no need to remove k8s service account", - objects: []runtime.Object{ - NewServiceAccount(kServiceAccountName, testNS, gServiceAccountName, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudPubSubSource", - UID: "test-pubsub-uid1", - Name: identifiableName, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }, { - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudPubSubSource", - UID: "test-pubsub-uid2", - Name: identifiableName + "new", - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - }, - }} - - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - cs := fakeKubeClient.NewSimpleClientset(tc.objects...) - iamClient := gclient.NewTestClient() - m, err := iam.NewIAMPolicyManager(ctx, iamClient) - if err != nil { - t.Fatal(err) - } - identity := &Identity{ - kubeClient: cs, - policyManager: m, - gcpAuthStore: gcpauth.NewStore(logtesting.TestLogger(t)), + gcpAuthStore: NewGCPAuthTestStore(t, tc.config), } identifiable := NewCloudPubSubSource(identifiableName, testNS, - WithCloudPubSubSourceGCPServiceAccount(gServiceAccountName), - WithCloudPubSubSourceServiceAccountName("test")) + WithCloudPubSubSourceServiceAccountName(kServiceAccountName)) + identifiable.Spec.ServiceAccountName = kServiceAccountName identifiable.SetAnnotations(map[string]string{ duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName, }) diff --git a/pkg/reconciler/identity/resources/service_account.go b/pkg/reconciler/identity/resources/service_account.go index 969c7b974b..926dc17fce 100644 --- a/pkg/reconciler/identity/resources/service_account.go +++ b/pkg/reconciler/identity/resources/service_account.go @@ -17,9 +17,6 @@ limitations under the License. package resources import ( - "fmt" - "strings" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -32,12 +29,6 @@ type IdentityNames struct { KServiceAccountName string GoogleServiceAccountName string Namespace string - ClusterName string -} - -// GenerateServiceAccountName generates a k8s ServiceAccount name according to GCP ServiceAccount -func GenerateServiceAccountName(gServiceAccount, clusterName string) string { - return fmt.Sprintf("%s-%s", strings.Split(gServiceAccount, "@")[0], clusterName) } // MakeServiceAccount creates a K8s ServiceAccount object for the Namespace. diff --git a/pkg/reconciler/identity/resources/service_account_test.go b/pkg/reconciler/identity/resources/service_account_test.go index 411af88202..ea801e6237 100644 --- a/pkg/reconciler/identity/resources/service_account_test.go +++ b/pkg/reconciler/identity/resources/service_account_test.go @@ -27,18 +27,8 @@ import ( var ( gServiceAccountName = "test@test.iam.gserviceaccount.com" kServiceAccountName = "test-cluster" - clusterName = "cluster" ) -func TestGenerateServiceAccountName(t *testing.T) { - want := kServiceAccountName - got := GenerateServiceAccountName(gServiceAccountName, clusterName) - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("unexpected (-want, +got) = %v", diff) - } -} - func TestMakeServiceAccount(t *testing.T) { want := &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ @@ -52,8 +42,7 @@ func TestMakeServiceAccount(t *testing.T) { got := MakeServiceAccount(IdentityNames{ KServiceAccountName: kServiceAccountName, GoogleServiceAccountName: gServiceAccountName, - Namespace: "default", - ClusterName: clusterName}) + Namespace: "default"}) if diff := cmp.Diff(want, got); diff != "" { t.Errorf("unexpected (-want, +got) = %v", diff) } diff --git a/pkg/reconciler/intevents/pullsubscription/reconciler.go b/pkg/reconciler/intevents/pullsubscription/reconciler.go index 80ec0985c0..49450dabe2 100644 --- a/pkg/reconciler/intevents/pullsubscription/reconciler.go +++ b/pkg/reconciler/intevents/pullsubscription/reconciler.go @@ -103,9 +103,9 @@ func (r *Base) ReconcileKind(ctx context.Context, ps *v1alpha1.PullSubscription) ps.Status.InitializeConditions() ps.Status.ObservedGeneration = ps.Generation - // If pullsubscription doesn't have ownerReference and GCP ServiceAccount is provided, reconcile workload identity. + // If pullsubscription doesn't have ownerReference and ServiceAccountName is provided, reconcile workload identity. // Otherwise, its owner will reconcile workload identity. - if (ps.OwnerReferences == nil || len(ps.OwnerReferences) == 0) && (ps.Spec.ServiceAccountName != "" || ps.Spec.GoogleServiceAccount != "") { + if (ps.OwnerReferences == nil || len(ps.OwnerReferences) == 0) && ps.Spec.ServiceAccountName != "" { if _, err := r.Identity.ReconcileWorkloadIdentity(ctx, ps.Spec.Project, ps); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile Pub/Sub subscription workload identity: %s", err.Error()) } @@ -402,9 +402,11 @@ func (r *Base) resolveDestination(ctx context.Context, destination duckv1.Destin } func (r *Base) FinalizeKind(ctx context.Context, ps *v1alpha1.PullSubscription) reconciler.Event { - // If pullsubscription doesn't have ownerReference, k8s ServiceAccount exists and it only has one ownerReference, remove the corresponding GCP ServiceAccount iam policy binding. + // If pullsubscription doesn't have ownerReference, and + // 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 (ps.OwnerReferences == nil || len(ps.OwnerReferences) == 0) && (ps.Spec.ServiceAccountName != "" || ps.Spec.GoogleServiceAccount != "") { + if (ps.OwnerReferences == nil || len(ps.OwnerReferences) == 0) && ps.Spec.ServiceAccountName != "" { if err := r.Identity.DeleteWorkloadIdentity(ctx, ps.Spec.Project, ps); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete delete Pub/Sub subscription workload identity: %s", err.Error()) } diff --git a/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter.go b/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter.go index 93ef18e90e..7fdff21943 100644 --- a/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter.go +++ b/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter.go @@ -26,10 +26,8 @@ import ( "knative.dev/pkg/kmeta" "knative.dev/pkg/logging" - duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" "github.com/google/knative-gcp/pkg/apis/intevents/v1alpha1" "github.com/google/knative-gcp/pkg/pubsub/adapter/converters" - "github.com/google/knative-gcp/pkg/reconciler/identity/resources" "github.com/google/knative-gcp/pkg/utils" "k8s.io/api/apps/v1" @@ -151,17 +149,6 @@ func makeReceiveAdapterPodSpec(ctx context.Context, args *ReceiveAdapterArgs) *c }}, } - // If GCP service account is specified, use that service account as credential. - if args.PullSubscription.Spec.GoogleServiceAccount != "" { - kServiceAccountName := resources.GenerateServiceAccountName(args.PullSubscription.Spec.GoogleServiceAccount, args.PullSubscription.Annotations[duckv1alpha1.ClusterNameAnnotation]) - return &corev1.PodSpec{ - ServiceAccountName: kServiceAccountName, - Containers: []corev1.Container{ - receiveAdapterContainer, - }, - } - } - // If there is no secret to embed, return what we have. if args.PullSubscription.Spec.Secret == nil { return &corev1.PodSpec{ diff --git a/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter_test.go b/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter_test.go index 8b855d4184..26cca038de 100644 --- a/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter_test.go +++ b/pkg/reconciler/intevents/pullsubscription/resources/receive_adapter_test.go @@ -341,8 +341,8 @@ func TestMakeFullReceiveAdapter(t *testing.T) { } } -func TestMakeReceiveAdapterWithGCPServiceAccount(t *testing.T) { - gServiceAccountName := "test@test.iam.gserviceaccount.com" +func TestMakeReceiveAdapterWithServiceAccount(t *testing.T) { + serviceAccountName := "test" ps := &v1alpha1.PullSubscription{ ObjectMeta: metav1.ObjectMeta{ Name: "testname", @@ -355,13 +355,7 @@ func TestMakeReceiveAdapterWithGCPServiceAccount(t *testing.T) { Spec: v1alpha1.PullSubscriptionSpec{ PubSubSpec: duckv1alpha1.PubSubSpec{ IdentitySpec: duckv1alpha1.IdentitySpec{ - GoogleServiceAccount: gServiceAccountName, - }, - Secret: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "eventing-secret-name", - }, - Key: "eventing-secret-key", + ServiceAccountName: serviceAccountName, }, Project: "eventing-name", SourceSpec: duckv1.SourceSpec{ @@ -427,7 +421,7 @@ func TestMakeReceiveAdapterWithGCPServiceAccount(t *testing.T) { }, }, Spec: corev1.PodSpec{ - ServiceAccountName: "test-fake-cluster-name", + ServiceAccountName: "test", Containers: []corev1.Container{{ Name: "receive-adapter", Image: "test-image", diff --git a/pkg/reconciler/intevents/resources/pullsubscription.go b/pkg/reconciler/intevents/resources/pullsubscription.go index d74bcb703e..17d557fb0b 100644 --- a/pkg/reconciler/intevents/resources/pullsubscription.go +++ b/pkg/reconciler/intevents/resources/pullsubscription.go @@ -51,8 +51,7 @@ func MakePullSubscription(args *PullSubscriptionArgs) *inteventsv1alpha1.PullSub Spec: inteventsv1alpha1.PullSubscriptionSpec{ PubSubSpec: duckv1alpha1.PubSubSpec{ IdentitySpec: duckv1alpha1.IdentitySpec{ - GoogleServiceAccount: args.Spec.IdentitySpec.GoogleServiceAccount, - ServiceAccountName: args.Spec.IdentitySpec.ServiceAccountName, + ServiceAccountName: args.Spec.IdentitySpec.ServiceAccountName, }, Secret: args.Spec.Secret, Project: args.Spec.Project, diff --git a/pkg/reconciler/intevents/resources/topic.go b/pkg/reconciler/intevents/resources/topic.go index 3acb60b6dc..4232322e12 100644 --- a/pkg/reconciler/intevents/resources/topic.go +++ b/pkg/reconciler/intevents/resources/topic.go @@ -48,8 +48,7 @@ func MakeTopic(args *TopicArgs) *inteventsv1alpha1.Topic { }, Spec: inteventsv1alpha1.TopicSpec{ IdentitySpec: duckv1alpha1.IdentitySpec{ - GoogleServiceAccount: args.Spec.IdentitySpec.GoogleServiceAccount, - ServiceAccountName: args.Spec.IdentitySpec.ServiceAccountName, + ServiceAccountName: args.Spec.IdentitySpec.ServiceAccountName, }, Secret: args.Spec.Secret, Project: args.Spec.Project, diff --git a/pkg/reconciler/intevents/topic/resources/publisher.go b/pkg/reconciler/intevents/topic/resources/publisher.go index 02f732d952..a133628cf0 100644 --- a/pkg/reconciler/intevents/topic/resources/publisher.go +++ b/pkg/reconciler/intevents/topic/resources/publisher.go @@ -24,9 +24,7 @@ import ( "knative.dev/pkg/kmeta" servingv1 "knative.dev/serving/pkg/apis/serving/v1" - duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" "github.com/google/knative-gcp/pkg/apis/intevents/v1alpha1" - "github.com/google/knative-gcp/pkg/reconciler/identity/resources" ) // PublisherArgs are the arguments needed to create a Topic publisher. @@ -70,17 +68,6 @@ func makePublisherPodSpec(args *PublisherArgs) *corev1.PodSpec { }}, } - // If GCP service account is specified, use that service account as credential. - if args.Topic.Spec.GoogleServiceAccount != "" { - kServiceAccountName := resources.GenerateServiceAccountName(args.Topic.Spec.GoogleServiceAccount, args.Topic.Annotations[duckv1alpha1.ClusterNameAnnotation]) - return &corev1.PodSpec{ - ServiceAccountName: kServiceAccountName, - Containers: []corev1.Container{ - publisherContainer, - }, - } - } - // If k8s service account is specified, use that service account as credential. if args.Topic.Spec.ServiceAccountName != "" { return &corev1.PodSpec{ diff --git a/pkg/reconciler/intevents/topic/resources/publisher_test.go b/pkg/reconciler/intevents/topic/resources/publisher_test.go index f324795a6e..d68699889f 100644 --- a/pkg/reconciler/intevents/topic/resources/publisher_test.go +++ b/pkg/reconciler/intevents/topic/resources/publisher_test.go @@ -138,8 +138,8 @@ func TestMakePublisher(t *testing.T) { } } -func TestMakePublisherWithGCPServiceAccount(t *testing.T) { - gServiceAccountName := "test@test.iam.gserviceaccount.com" +func TestMakePublisherWithServiceAccount(t *testing.T) { + serviceAccountName := "test" topic := &v1alpha1.Topic{ ObjectMeta: metav1.ObjectMeta{ Name: "topic-name", @@ -152,7 +152,7 @@ func TestMakePublisherWithGCPServiceAccount(t *testing.T) { Project: "eventing-name", Topic: "topic-name", IdentitySpec: duckv1alpha1.IdentitySpec{ - GoogleServiceAccount: gServiceAccountName, + ServiceAccountName: serviceAccountName, }, }, } @@ -208,7 +208,7 @@ func TestMakePublisherWithGCPServiceAccount(t *testing.T) { Value: "TracingConfig-ABC123", }}, }}, - ServiceAccountName: "test-fake-cluster-name", + ServiceAccountName: "test", }, }, }}, diff --git a/pkg/reconciler/intevents/topic/topic.go b/pkg/reconciler/intevents/topic/topic.go index e9b267a123..f9a5ae9dc5 100644 --- a/pkg/reconciler/intevents/topic/topic.go +++ b/pkg/reconciler/intevents/topic/topic.go @@ -91,9 +91,9 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, topic *v1alpha1.Topic) r topic.Status.InitializeConditions() topic.Status.ObservedGeneration = topic.Generation - // If topic doesn't have ownerReference and GCP ServiceAccount is provided, reconcile workload identity. + // If topic doesn't have ownerReference and ServiceAccountName is provided, reconcile workload identity. // Otherwise, its owner will reconcile workload identity. - if (topic.OwnerReferences == nil || len(topic.OwnerReferences) == 0) && (topic.Spec.ServiceAccountName != "" || topic.Spec.GoogleServiceAccount != "") { + if (topic.OwnerReferences == nil || len(topic.OwnerReferences) == 0) && topic.Spec.ServiceAccountName != "" { if _, err := r.Identity.ReconcileWorkloadIdentity(ctx, topic.Spec.Project, topic); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile Pub/Sub topic workload identity: %s", err.Error()) } @@ -276,9 +276,11 @@ func (r *Reconciler) UpdateFromTracingConfigMap(cfg *corev1.ConfigMap) { } func (r *Reconciler) FinalizeKind(ctx context.Context, topic *v1alpha1.Topic) reconciler.Event { - // If topic doesn't have ownerReference, k8s ServiceAccount exists and it only has one ownerReference, remove the corresponding GCP ServiceAccount iam policy binding. + // If topic doesn't have ownerReference, and + // 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 (topic.OwnerReferences == nil || len(topic.OwnerReferences) == 0) && (topic.Spec.ServiceAccountName != "" || topic.Spec.GoogleServiceAccount != "") { + if (topic.OwnerReferences == nil || len(topic.OwnerReferences) == 0) && topic.Spec.ServiceAccountName != "" { if err := r.Identity.DeleteWorkloadIdentity(ctx, topic.Spec.Project, topic); err != nil { return reconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete delete Pub/Sub topic workload identity: %s", err.Error()) } diff --git a/pkg/reconciler/messaging/channel/channel.go b/pkg/reconciler/messaging/channel/channel.go index 2f4a2a1530..55f88fb1bd 100644 --- a/pkg/reconciler/messaging/channel/channel.go +++ b/pkg/reconciler/messaging/channel/channel.go @@ -73,8 +73,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, channel *v1alpha1.Channe channel.Status.InitializeConditions() channel.Status.ObservedGeneration = channel.Generation - // If GCP ServiceAccount is provided, reconcile workload identity. - if channel.Spec.ServiceAccountName != "" || channel.Spec.GoogleServiceAccount != "" { + // If ServiceAccountName is provided, reconcile workload identity. + if channel.Spec.ServiceAccountName != "" { if _, err := r.Identity.ReconcileWorkloadIdentity(ctx, channel.Spec.Project, channel); err != nil { return pkgreconciler.NewEvent(corev1.EventTypeWarning, workloadIdentityFailed, "Failed to reconcile Channel workload identity: %s", err.Error()) } @@ -163,7 +163,6 @@ func (r *Reconciler) syncSubscribers(ctx context.Context, channel *v1alpha1.Chan Name: genName, Project: channel.Spec.Project, Topic: channel.Status.TopicID, - ServiceAccount: channel.Spec.GoogleServiceAccount, ServiceAccountName: channel.Spec.ServiceAccountName, Secret: channel.Spec.Secret, Labels: resources.GetPullSubscriptionLabels(controllerAgentName, channel.Name, genName, string(channel.UID)), @@ -199,7 +198,6 @@ func (r *Reconciler) syncSubscribers(ctx context.Context, channel *v1alpha1.Chan Name: genName, Project: channel.Spec.Project, Topic: channel.Status.TopicID, - ServiceAccount: channel.Spec.GoogleServiceAccount, ServiceAccountName: channel.Spec.ServiceAccountName, Secret: channel.Spec.Secret, Labels: resources.GetPullSubscriptionLabels(controllerAgentName, channel.Name, genName, string(channel.UID)), @@ -314,7 +312,6 @@ func (r *Reconciler) reconcileTopic(ctx context.Context, channel *v1alpha1.Chann Owner: channel, Name: resources.GeneratePublisherName(channel), Project: channel.Spec.Project, - ServiceAccount: channel.Spec.GoogleServiceAccount, ServiceAccountName: channel.Spec.ServiceAccountName, Secret: channel.Spec.Secret, Topic: resources.GenerateTopicID(channel), @@ -379,9 +376,10 @@ func (r *Reconciler) getPullSubscriptionStatus(ps *inteventsv1alpha1.PullSubscri } func (r *Reconciler) FinalizeKind(ctx context.Context, channel *v1alpha1.Channel) pkgreconciler.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 channel.Spec.ServiceAccountName != "" || channel.Spec.GoogleServiceAccount != "" { + if channel.Spec.ServiceAccountName != "" { if err := r.Identity.DeleteWorkloadIdentity(ctx, channel.Spec.Project, channel); err != nil { return pkgreconciler.NewEvent(corev1.EventTypeWarning, deleteWorkloadIdentityFailed, "Failed to delete Channel workload identity: %s", err.Error()) } diff --git a/pkg/reconciler/messaging/channel/channel_test.go b/pkg/reconciler/messaging/channel/channel_test.go index ef42fd0212..35f2c29b1b 100644 --- a/pkg/reconciler/messaging/channel/channel_test.go +++ b/pkg/reconciler/messaging/channel/channel_test.go @@ -504,45 +504,6 @@ func TestAllCases(t *testing.T) { WantPatches: []clientgotesting.PatchActionImpl{ patchFinalizers(testNS, channelName, true), }, - }, { - Name: "delete channel failed with getting k8s service account error", - Objects: []runtime.Object{ - NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelGCPServiceAccount(gServiceAccount), - WithChannelDefaults, - WithChannelDeletionTimestamp, - WithChannelServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithChannelAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName}), - ), - }, - Key: testNS + "/" + channelName, - WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", - `Failed to delete Channel workload identity: getting k8s service account failed with: serviceaccounts "test123-fake-cluster-name" not found`), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithInitChannelConditions, - WithChannelGCPServiceAccount(gServiceAccount), - WithChannelDefaults, - WithChannelServiceAccountName("test123-"+testingMetadataClient.FakeClusterName), - WithChannelDeletionTimestamp, - WithChannelWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", - `serviceaccounts "test123-fake-cluster-name" not found`), - WithChannelAnnotations(map[string]string{ - duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName}), - ), - }}, }} defer logtesting.ClearAll() diff --git a/pkg/reconciler/messaging/channel/resources/pullsubscription.go b/pkg/reconciler/messaging/channel/resources/pullsubscription.go index c96a111ad0..3aedb770cb 100644 --- a/pkg/reconciler/messaging/channel/resources/pullsubscription.go +++ b/pkg/reconciler/messaging/channel/resources/pullsubscription.go @@ -50,8 +50,7 @@ func MakePullSubscription(args *PullSubscriptionArgs) *v1alpha1.PullSubscription PubSubSpec: gcpduckv1alpha1.PubSubSpec{ SourceSpec: duckv1.SourceSpec{}, IdentitySpec: gcpduckv1alpha1.IdentitySpec{ - GoogleServiceAccount: args.ServiceAccount, - ServiceAccountName: args.ServiceAccountName, + ServiceAccountName: args.ServiceAccountName, }, Secret: args.Secret, Project: args.Project, diff --git a/pkg/reconciler/messaging/channel/resources/topic.go b/pkg/reconciler/messaging/channel/resources/topic.go index f90245f666..372e522c02 100644 --- a/pkg/reconciler/messaging/channel/resources/topic.go +++ b/pkg/reconciler/messaging/channel/resources/topic.go @@ -52,8 +52,7 @@ func MakeTopic(args *TopicArgs) *v1alpha1.Topic { }, Spec: v1alpha1.TopicSpec{ IdentitySpec: duckv1alpha1.IdentitySpec{ - GoogleServiceAccount: args.ServiceAccount, - ServiceAccountName: args.ServiceAccountName, + ServiceAccountName: args.ServiceAccountName, }, Secret: args.Secret, Project: args.Project, diff --git a/pkg/reconciler/testing/auditlogs.go b/pkg/reconciler/testing/auditlogs.go index b28b5db091..1ae47aedb4 100644 --- a/pkg/reconciler/testing/auditlogs.go +++ b/pkg/reconciler/testing/auditlogs.go @@ -171,12 +171,6 @@ func WithCloudAuditLogsSourceServiceName(serviceName string) CloudAuditLogsSourc } } -func WithCloudAuditLogsSourceGCPServiceAccount(gServiceAccount string) CloudAuditLogsSourceOption { - return func(s *v1alpha1.CloudAuditLogsSource) { - s.Spec.GoogleServiceAccount = gServiceAccount - } -} - func WithCloudAuditLogsSourceServiceAccount(kServiceAccount string) CloudAuditLogsSourceOption { return func(s *v1alpha1.CloudAuditLogsSource) { s.Spec.ServiceAccountName = kServiceAccount diff --git a/pkg/reconciler/testing/build.go b/pkg/reconciler/testing/build.go index cddf636823..eee3a6cb4b 100644 --- a/pkg/reconciler/testing/build.go +++ b/pkg/reconciler/testing/build.go @@ -62,12 +62,6 @@ func WithCloudBuildSourceSink(gvk metav1.GroupVersionKind, name string) CloudBui } } -func WithCloudBuildSourceGCPServiceAccount(gServiceAccount string) CloudBuildSourceOption { - return func(bs *v1alpha1.CloudBuildSource) { - bs.Spec.GoogleServiceAccount = gServiceAccount - } -} - func WithCloudBuildSourceDeletionTimestamp(s *v1alpha1.CloudBuildSource) { t := metav1.NewTime(time.Unix(1e9, 0)) s.ObjectMeta.SetDeletionTimestamp(&t) diff --git a/pkg/reconciler/testing/channel.go b/pkg/reconciler/testing/channel.go index 7c4c331a22..ddf215f91b 100644 --- a/pkg/reconciler/testing/channel.go +++ b/pkg/reconciler/testing/channel.go @@ -129,12 +129,6 @@ func WithChannelDefaults(s *v1alpha1.Channel) { s.SetDefaults(gcpauthtesthelper.ContextWithDefaults()) } -func WithChannelGCPServiceAccount(gServiceAccount string) ChannelOption { - return func(ps *v1alpha1.Channel) { - ps.Spec.GoogleServiceAccount = gServiceAccount - } -} - func WithChannelServiceAccount(kServiceAccount string) ChannelOption { return func(ps *v1alpha1.Channel) { ps.Spec.ServiceAccountName = kServiceAccount diff --git a/pkg/reconciler/testing/pubsub.go b/pkg/reconciler/testing/pubsub.go index 133c867f1d..62b63900b0 100644 --- a/pkg/reconciler/testing/pubsub.go +++ b/pkg/reconciler/testing/pubsub.go @@ -60,12 +60,6 @@ func WithCloudPubSubSourceSink(gvk metav1.GroupVersionKind, name string) CloudPu } } -func WithCloudPubSubSourceGCPServiceAccount(gServiceAccount string) CloudPubSubSourceOption { - return func(ps *v1alpha1.CloudPubSubSource) { - ps.Spec.GoogleServiceAccount = gServiceAccount - } -} - func WithCloudPubSubSourceServiceAccount(kServiceAccount string) CloudPubSubSourceOption { return func(ps *v1alpha1.CloudPubSubSource) { ps.Spec.ServiceAccountName = kServiceAccount diff --git a/pkg/reconciler/testing/scheduler.go b/pkg/reconciler/testing/scheduler.go index b0bb808330..0336963509 100644 --- a/pkg/reconciler/testing/scheduler.go +++ b/pkg/reconciler/testing/scheduler.go @@ -78,12 +78,6 @@ func WithCloudSchedulerSourceSchedule(schedule string) CloudSchedulerSourceOptio } } -func WithCloudSchedulerSourceGCPServiceAccount(gServiceAccount string) CloudSchedulerSourceOption { - return func(ps *v1alpha1.CloudSchedulerSource) { - ps.Spec.GoogleServiceAccount = gServiceAccount - } -} - func WithCloudSchedulerSourceServiceAccount(kServiceAccount string) CloudSchedulerSourceOption { return func(ps *v1alpha1.CloudSchedulerSource) { ps.Spec.ServiceAccountName = kServiceAccount diff --git a/pkg/reconciler/testing/storage.go b/pkg/reconciler/testing/storage.go index e227010f8e..4c3b70b1d6 100644 --- a/pkg/reconciler/testing/storage.go +++ b/pkg/reconciler/testing/storage.go @@ -107,12 +107,6 @@ func WithCloudStorageSourceWorkloadIdentityFailed(reason, message string) CloudS } } -func WithCloudStorageSourceGCPServiceAccount(gServiceAccount string) CloudStorageSourceOption { - return func(ps *v1alpha1.CloudStorageSource) { - ps.Spec.GoogleServiceAccount = gServiceAccount - } -} - func WithCloudStorageSourceServiceAccount(kServiceAccount string) CloudStorageSourceOption { return func(ps *v1alpha1.CloudStorageSource) { ps.Spec.ServiceAccountName = kServiceAccount