Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Remove spec.googleServiceAccount in v1alpha1 #1249

Merged
merged 2 commits into from
Jun 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions config/core/resources/channel.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >
Expand Down
6 changes: 0 additions & 6 deletions config/core/resources/cloudauditlogssource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >
Expand Down
6 changes: 0 additions & 6 deletions config/core/resources/cloudbuildsource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >
Expand Down
6 changes: 0 additions & 6 deletions config/core/resources/cloudpubsubsource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >
Expand Down
6 changes: 0 additions & 6 deletions config/core/resources/cloudschedulersource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >
Expand Down
6 changes: 0 additions & 6 deletions config/core/resources/cloudstoragesource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: >
Expand Down
3 changes: 0 additions & 3 deletions config/core/resources/pullsubscription.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
3 changes: 0 additions & 3 deletions config/core/resources/topic.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
33 changes: 4 additions & 29 deletions pkg/apis/duck/v1alpha1/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
grac3gao-zz marked this conversation as resolved.
Show resolved Hide resolved
return validateK8sServiceAccount(kServiceAccountName)
}
return nil
}

func validateSecret(secret *corev1.SecretKeySelector) *apis.FieldError {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/duck/v1alpha1/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions pkg/apis/duck/v1alpha1/identity_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/events/v1alpha1/cloudbuildsource_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/events/v1alpha1/cloudpubsubsource_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/events/v1alpha1/cloudschedulersource_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/events/v1alpha1/cloudstoragesource_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/events/v1alpha1/cloudstoragesource_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/messaging/v1alpha1/channel_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/messaging/v1alpha1/channel_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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"},
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/messaging/v1alpha1/channel_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 5 additions & 4 deletions pkg/reconciler/events/auditlogs/auditlogs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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())
}
Expand Down
Loading