From f1babc017153da828143030be85b699e3d3cdd81 Mon Sep 17 00:00:00 2001 From: Grace Gao <52978759+grac3gao@users.noreply.github.com> Date: Fri, 27 Mar 2020 12:12:52 -0700 Subject: [PATCH] Add Workload Identity status condition (#714) * status * update code * hack/update-codegen.sh + UT coverage * fix typo * update code * update code * update code * update code --- pkg/apis/duck/v1alpha1/identity_lifecycle.go | 35 ++++ .../duck/v1alpha1/identity_lifecycle_test.go | 55 +++++++ pkg/apis/duck/v1alpha1/identity_types.go | 54 +++++++ pkg/apis/duck/v1alpha1/identity_types_test.go | 33 ++++ pkg/apis/duck/v1alpha1/pubsub_types.go | 23 ++- .../duck/v1alpha1/zz_generated.deepcopy.go | 48 +++++- .../cloudauditlogssource_lifecycle.go | 33 ---- .../cloudauditlogssource_lifecycle_test.go | 72 ++++----- .../v1alpha1/cloudauditlogssource_types.go | 28 ++-- .../cloudauditlogssource_types_test.go | 21 ++- .../cloudauditlogssource_validation_test.go | 4 +- .../v1alpha1/cloudpubsubsource_types.go | 36 +++-- .../v1alpha1/cloudpubsubsource_types_test.go | 45 +++++- .../cloudpubsubsource_validation_test.go | 4 +- .../cloudschedulersource_lifecycle.go | 38 ----- .../cloudschedulersource_lifecycle_test.go | 78 ++++----- .../v1alpha1/cloudschedulersource_types.go | 26 +-- .../cloudschedulersource_types_test.go | 21 ++- .../cloudschedulersource_validation_test.go | 16 +- .../v1alpha1/cloudstoragesource_lifecycle.go | 34 ---- .../cloudstoragesource_lifecycle_test.go | 72 ++++----- .../v1alpha1/cloudstoragesource_types.go | 28 ++-- .../v1alpha1/cloudstoragesource_types_test.go | 21 ++- .../cloudstoragesource_validation_test.go | 20 ++- .../events/v1alpha1/zz_generated.deepcopy.go | 2 +- .../v1alpha1/channel_lifecycle_test.go | 149 ++++++++++-------- pkg/apis/messaging/v1alpha1/channel_types.go | 29 ++-- .../messaging/v1alpha1/channel_types_test.go | 48 +++++- .../v1alpha1/channel_validation_test.go | 20 ++- .../v1alpha1/zz_generated.deepcopy.go | 3 +- pkg/duck/identifiable.go | 15 +- pkg/duck/pubsubable.go | 11 +- .../events/auditlogs/auditlogs_test.go | 62 ++------ pkg/reconciler/events/pubsub/pubsub_test.go | 64 ++------ .../events/scheduler/scheduler_test.go | 68 +++----- pkg/reconciler/events/storage/storage_test.go | 63 ++------ pkg/reconciler/identity/reconciler.go | 25 ++- .../messaging/channel/channel_test.go | 66 ++------ pkg/reconciler/testing/auditlogs.go | 18 ++- pkg/reconciler/testing/channel.go | 6 + pkg/reconciler/testing/pubsub.go | 6 + pkg/reconciler/testing/scheduler.go | 20 ++- pkg/reconciler/testing/storage.go | 18 ++- 43 files changed, 869 insertions(+), 669 deletions(-) create mode 100644 pkg/apis/duck/v1alpha1/identity_lifecycle.go create mode 100644 pkg/apis/duck/v1alpha1/identity_lifecycle_test.go create mode 100644 pkg/apis/duck/v1alpha1/identity_types.go create mode 100644 pkg/apis/duck/v1alpha1/identity_types_test.go diff --git a/pkg/apis/duck/v1alpha1/identity_lifecycle.go b/pkg/apis/duck/v1alpha1/identity_lifecycle.go new file mode 100644 index 0000000000..62cdbda1b4 --- /dev/null +++ b/pkg/apis/duck/v1alpha1/identity_lifecycle.go @@ -0,0 +1,35 @@ +/* +Copyright 2020 Google LLC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "knative.dev/pkg/apis" +) + +func (s *IdentityStatus) MarkWorkloadIdentityConfigured(cs *apis.ConditionSet) { + cs.Manage(s).MarkTrue(IdentityConfigured) +} + +func (s *IdentityStatus) MarkWorkloadIdentityNotConfigured(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { + cs.Manage(s).MarkUnknown(IdentityConfigured, reason, messageFormat, messageA...) +} + +func (s *IdentityStatus) MarkWorkloadIdentityFailed(cs *apis.ConditionSet, reason, messageFormat string, messageA ...interface{}) { + cs.Manage(s).MarkFalse(IdentityConfigured, reason, messageFormat, messageA...) + // Set ConditionReady to be false. + // ConditionType IdentityConfigured is not included in apis.NewLivingConditionSet{}, so it is not counted for conditionReady. + // This is because if Workload Identity is not enabled, IdentityConfigured will be unknown. + // It will be counted for conditionReady only if it is failed. + cs.Manage(s).MarkFalse(apis.ConditionReady, "WorkloadIdentityFailed", messageFormat, messageA...) +} diff --git a/pkg/apis/duck/v1alpha1/identity_lifecycle_test.go b/pkg/apis/duck/v1alpha1/identity_lifecycle_test.go new file mode 100644 index 0000000000..31d7d50fb7 --- /dev/null +++ b/pkg/apis/duck/v1alpha1/identity_lifecycle_test.go @@ -0,0 +1,55 @@ +/* +Copyright 2020 Google LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "knative.dev/pkg/apis" + "testing" +) + +func TestMarkWorkloadIdentityConfigured(t *testing.T) { + status := &IdentityStatus{} + condSet := apis.NewLivingConditionSet() + status.MarkWorkloadIdentityConfigured(&condSet) + got := status.IsReady() + want := true + if got != want { + t.Errorf("unexpected readiness: want %v, got %v", want, got) + } +} + +func TestMarkWorkloadIdentityNotConfigured(t *testing.T) { + status := &IdentityStatus{} + condSet := apis.NewLivingConditionSet() + status.MarkWorkloadIdentityFailed(&condSet, "failed", "failed") + got := status.IsReady() + want := false + if got != want { + t.Errorf("unexpected readiness: want %v, got %v", want, got) + } +} + +func TestMarkWorkloadIdentityFailed(t *testing.T) { + status := &IdentityStatus{} + condSet := apis.NewLivingConditionSet() + status.MarkWorkloadIdentityNotConfigured(&condSet, "failed", "failed") + got := status.IsReady() + want := true + if got != false { + t.Errorf("unexpected readiness: want %v, got %v", want, got) + } +} diff --git a/pkg/apis/duck/v1alpha1/identity_types.go b/pkg/apis/duck/v1alpha1/identity_types.go new file mode 100644 index 0000000000..6515692011 --- /dev/null +++ b/pkg/apis/duck/v1alpha1/identity_types.go @@ -0,0 +1,54 @@ +/* +Copyright 2020 Google LLC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" +) + +type IdentitySpec struct { + // ServiceAccount 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 + // TODO rename ServiceAccount, issue https://github.com/google/knative-gcp/issues/723 + ServiceAccount string `json:"serviceAccount,omitempty"` +} + +// IdentityStatus inherits duck/v1 Status and adds a ServiceAccountName. +type IdentityStatus struct { + // Inherits duck/v1 Status,, which currently provides: + // * ObservedGeneration - the 'Generation' of the Service that was last processed by the controller. + // * Conditions - the latest available observations of a resource's current state. + duckv1.Status `json:",inline"` + // ServiceAccountName is the k8s service account associated with Google service account. + ServiceAccountName string `json:"serviceAccountName,omitempty"` +} + +const ( + IdentityConfigured apis.ConditionType = "WorkloadIdentityConfigured" +) + +// IsReady returns true if the resource is ready overall. +func (ss *IdentityStatus) IsReady() bool { + for _, c := range ss.Conditions { + switch c.Type { + // Look for the "happy" condition, which is the only condition that + // we can reliably understand to be the overall state of the resource. + case apis.ConditionReady, apis.ConditionSucceeded: + return c.IsTrue() + } + } + return false +} diff --git a/pkg/apis/duck/v1alpha1/identity_types_test.go b/pkg/apis/duck/v1alpha1/identity_types_test.go new file mode 100644 index 0000000000..33fb55e17c --- /dev/null +++ b/pkg/apis/duck/v1alpha1/identity_types_test.go @@ -0,0 +1,33 @@ +/* +Copyright 2020 Google LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "github.com/google/go-cmp/cmp" + "testing" +) + +func TestIsReady(t *testing.T) { + status := &IdentityStatus{} + want := false + + got := status.IsReady() + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Unexpected difference (-want, +got): %v", diff) + } +} diff --git a/pkg/apis/duck/v1alpha1/pubsub_types.go b/pkg/apis/duck/v1alpha1/pubsub_types.go index 48b7180a25..4fdc2674da 100644 --- a/pkg/apis/duck/v1alpha1/pubsub_types.go +++ b/pkg/apis/duck/v1alpha1/pubsub_types.go @@ -50,10 +50,7 @@ type PubSubSpec struct { // This brings in CloudEventOverrides and Sink. duckv1.SourceSpec `json:",inline"` - // ServiceAccount 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 - ServiceAccount string `json:"serviceAccount,omitempty"` + IdentitySpec `json:",inline"` // Secret is the credential to use to poll from a Cloud Pub/Sub subscription. // If not specified, defaults to: @@ -71,14 +68,16 @@ type PubSubSpec struct { // PubSubStatus shows how we expect folks to embed Addressable in // their Status field. type PubSubStatus struct { - // inherits duck/v1 SourceStatus, which currently provides: - // * ObservedGeneration - the 'Generation' of the Service that was last - // processed by the controller. - // * Conditions - the latest available observations of a resource's current - // state. - // * SinkURI - the current active sink URI that has been configured for the - // Source. - duckv1.SourceStatus `json:",inline"` + IdentityStatus `json:",inline"` + + // SinkURI is the current active sink URI that has been configured for the Source. + // +optional + SinkURI *apis.URL `json:"sinkUri,omitempty"` + + // CloudEventAttributes are the specific attributes that the Source uses + // as part of its CloudEvents. + // +optional + CloudEventAttributes []duckv1.CloudEventAttributes `json:"ceAttributes,omitempty"` // ProjectID is the project ID of the Topic, might have been resolved. // +optional diff --git a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go index b750223fc7..1be4d2e5a5 100644 --- a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go @@ -23,8 +23,43 @@ package v1alpha1 import ( v1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" + apis "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IdentitySpec) DeepCopyInto(out *IdentitySpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IdentitySpec. +func (in *IdentitySpec) DeepCopy() *IdentitySpec { + if in == nil { + return nil + } + out := new(IdentitySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IdentityStatus) DeepCopyInto(out *IdentityStatus) { + *out = *in + in.Status.DeepCopyInto(&out.Status) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IdentityStatus. +func (in *IdentityStatus) DeepCopy() *IdentityStatus { + if in == nil { + return nil + } + out := new(IdentityStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PubSub) DeepCopyInto(out *PubSub) { *out = *in @@ -90,6 +125,7 @@ func (in *PubSubList) DeepCopyObject() runtime.Object { func (in *PubSubSpec) DeepCopyInto(out *PubSubSpec) { *out = *in in.SourceSpec.DeepCopyInto(&out.SourceSpec) + out.IdentitySpec = in.IdentitySpec if in.Secret != nil { in, out := &in.Secret, &out.Secret *out = new(v1.SecretKeySelector) @@ -111,7 +147,17 @@ func (in *PubSubSpec) DeepCopy() *PubSubSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PubSubStatus) DeepCopyInto(out *PubSubStatus) { *out = *in - in.SourceStatus.DeepCopyInto(&out.SourceStatus) + in.IdentityStatus.DeepCopyInto(&out.IdentityStatus) + if in.SinkURI != nil { + in, out := &in.SinkURI, &out.SinkURI + *out = new(apis.URL) + (*in).DeepCopyInto(*out) + } + if in.CloudEventAttributes != nil { + in, out := &in.CloudEventAttributes, &out.CloudEventAttributes + *out = make([]duckv1.CloudEventAttributes, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle.go b/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle.go index 372cea8fac..6f8ea64fb7 100644 --- a/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle.go +++ b/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle.go @@ -17,7 +17,6 @@ limitations under the License. package v1alpha1 import ( - duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" "knative.dev/pkg/apis" ) @@ -41,38 +40,6 @@ func (s *CloudAuditLogsSourceStatus) InitializeConditions() { auditLogsSourceCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionFailed sets the condition that the status of underlying PullSubscription -// is False and why. -func (s *CloudAuditLogsSourceStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { - auditLogsSourceCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) -} - -// MarkPullSubscriptionUnknown sets the condition that the status of underlying PullSubscription -// is Unknown and why. -func (s *CloudAuditLogsSourceStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { - auditLogsSourceCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) -} - -// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready. -func (s *CloudAuditLogsSourceStatus) MarkPullSubscriptionReady() { - auditLogsSourceCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) -} - -// MarkTopicFailed sets the condition that the status of PubSub topic is False and why. -func (s *CloudAuditLogsSourceStatus) MarkTopicFailed(reason, messageFormat string, messageA ...interface{}) { - auditLogsSourceCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) -} - -// MarkTopicUnknown sets the condition that the status of PubSub topic is Unknown and why. -func (s *CloudAuditLogsSourceStatus) MarkTopicUnknown(reason, messageFormat string, messageA ...interface{}) { - auditLogsSourceCondSet.Manage(s).MarkUnknown(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) -} - -// MarkTopicReady sets the condition that the underlying PubSub topic was created successfully. -func (s *CloudAuditLogsSourceStatus) MarkTopicReady() { - auditLogsSourceCondSet.Manage(s).MarkTrue(duckv1alpha1.TopicReady) -} - // MarkSinkNotReady sets the condition that a CloudAuditLogsSource pubsub sink // has not been configured and why. func (s *CloudAuditLogsSourceStatus) MarkSinkNotReady(reason, messageFormat string, messageA ...interface{}) { diff --git a/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle_test.go b/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle_test.go index 7e7d867049..45baef0746 100644 --- a/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/cloudauditlogssource_lifecycle_test.go @@ -47,24 +47,24 @@ func TestCloudAuditLogsSourceStatusIsReady(t *testing.T) { }, { name: "the status of topic is false", s: func() *CloudAuditLogsSourceStatus { - s := &CloudAuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkPullSubscriptionReady() - s.MarkSinkReady() - s.MarkTopicFailed("test", "the status of topic is false") - return s + s := &CloudAuditLogsSource{} + s.Status.InitializeConditions() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkSinkReady() + s.Status.MarkTopicFailed(s.ConditionSet(), "test", "the status of topic is false") + return &s.Status }(), wantConditionStatus: corev1.ConditionFalse, want: false, }, { name: "the status of topic is unknown", s: func() *CloudAuditLogsSourceStatus { - s := &CloudAuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkPullSubscriptionReady() - s.MarkSinkReady() - s.MarkTopicUnknown("test", "the status of topic is unknown") - return s + s := &CloudAuditLogsSource{} + s.Status.InitializeConditions() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkSinkReady() + s.Status.MarkTopicUnknown(s.ConditionSet(), "test", "the status of topic is unknown") + return &s.Status }(), wantConditionStatus: corev1.ConditionUnknown, want: false, @@ -72,23 +72,23 @@ func TestCloudAuditLogsSourceStatusIsReady(t *testing.T) { { name: "the status of pullsubscription is false", s: func() *CloudAuditLogsSourceStatus { - s := &CloudAuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkSinkReady() - s.MarkPullSubscriptionFailed("test", "the status of pullsubscription is false") - return s + s := &CloudAuditLogsSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkSinkReady() + s.Status.MarkPullSubscriptionFailed(s.ConditionSet(), "test", "the status of pullsubscription is false") + return &s.Status }(), wantConditionStatus: corev1.ConditionFalse, }, { name: "the status of pullsubscription is unknown", s: func() *CloudAuditLogsSourceStatus { - s := &CloudAuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkSinkReady() - s.MarkPullSubscriptionUnknown("test", "the status of pullsubscription is unknown") - return s + s := &CloudAuditLogsSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkSinkReady() + s.Status.MarkPullSubscriptionUnknown(s.ConditionSet(), "test", "the status of pullsubscription is unknown") + return &s.Status }(), wantConditionStatus: corev1.ConditionUnknown, want: false, @@ -96,24 +96,24 @@ func TestCloudAuditLogsSourceStatusIsReady(t *testing.T) { { name: "sink is not ready", s: func() *CloudAuditLogsSourceStatus { - s := &CloudAuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionReady() - s.MarkSinkNotReady("test", "sink is not ready") - return s + s := &CloudAuditLogsSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkSinkNotReady("test", "sink is not ready") + return &s.Status }(), wantConditionStatus: corev1.ConditionFalse, want: false, }, { name: "ready", s: func() *CloudAuditLogsSourceStatus { - s := &CloudAuditLogsSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionReady() - s.MarkSinkReady() - return s + s := &CloudAuditLogsSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkSinkReady() + return &s.Status }(), wantConditionStatus: corev1.ConditionTrue, want: true, diff --git a/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go b/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go index 4850e0df5e..00fdc88c14 100644 --- a/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go +++ b/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go @@ -107,16 +107,15 @@ func (*CloudAuditLogsSource) GetGroupVersionKind() schema.GroupVersionKind { return SchemeGroupVersion.WithKind("CloudAuditLogsSource") } -///Methods for pubsubable interface - -// PubSubSpec returns the PubSubSpec portion of the Spec. -func (s *CloudAuditLogsSource) PubSubSpec() *duckv1alpha1.PubSubSpec { - return &s.Spec.PubSubSpec +// Methods for identifiable interface. +// IdentitySpec returns the IdentitySpec portion of the Spec. +func (s *CloudAuditLogsSource) IdentitySpec() *duckv1alpha1.IdentitySpec { + return &s.Spec.IdentitySpec } -// PubSubStatus returns the PubSubStatus portion of the Status. -func (s *CloudAuditLogsSource) PubSubStatus() *duckv1alpha1.PubSubStatus { - return &s.Status.PubSubStatus +// IdentityStatus returns the IdentityStatus portion of the Status. +func (s *CloudAuditLogsSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -124,9 +123,16 @@ func (*CloudAuditLogsSource) ConditionSet() *apis.ConditionSet { return &auditLogsSourceCondSet } -// Methods for identifiable interface -func (s *CloudAuditLogsSource) GetIdentity() string { - return s.Spec.ServiceAccount +///Methods for pubsubable interface. + +// PubSubSpec returns the PubSubSpec portion of the Spec. +func (s *CloudAuditLogsSource) PubSubSpec() *duckv1alpha1.PubSubSpec { + return &s.Spec.PubSubSpec +} + +// PubSubStatus returns the PubSubStatus portion of the Status. +func (s *CloudAuditLogsSource) PubSubStatus() *duckv1alpha1.PubSubStatus { + return &s.Status.PubSubStatus } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/events/v1alpha1/cloudauditlogssource_types_test.go b/pkg/apis/events/v1alpha1/cloudauditlogssource_types_test.go index 3497cb5293..e6d89bc80b 100644 --- a/pkg/apis/events/v1alpha1/cloudauditlogssource_types_test.go +++ b/pkg/apis/events/v1alpha1/cloudauditlogssource_types_test.go @@ -88,16 +88,31 @@ func TestCloudAuditLogsSourceEventID(t *testing.T) { } } -func TestCloudAuditLogsSourceGetIdentity(t *testing.T) { +func TestCloudAuditLogsSourceIdentitySpec(t *testing.T) { s := &CloudAuditLogsSource{ Spec: CloudAuditLogsSourceSpec{ PubSubSpec: duckv1alpha1.PubSubSpec{ - ServiceAccount: "test@test", + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: "test@test", + }, }, }, } want := "test@test" - got := s.GetIdentity() + got := s.IdentitySpec().ServiceAccount + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("failed to get expected (-want, +got) = %v", diff) + } +} + +func TestCloudAuditLogsSourceIdentityStatus(t *testing.T) { + s := &CloudAuditLogsSource{ + Status: CloudAuditLogsSourceStatus{ + PubSubStatus: duckv1alpha1.PubSubStatus{}, + }, + } + want := &duckv1alpha1.IdentityStatus{} + got := s.IdentityStatus() if diff := cmp.Diff(want, got); diff != "" { t.Errorf("failed to get expected (-want, +got) = %v", diff) } diff --git a/pkg/apis/events/v1alpha1/cloudauditlogssource_validation_test.go b/pkg/apis/events/v1alpha1/cloudauditlogssource_validation_test.go index 57a2bf858c..7481a757e8 100644 --- a/pkg/apis/events/v1alpha1/cloudauditlogssource_validation_test.go +++ b/pkg/apis/events/v1alpha1/cloudauditlogssource_validation_test.go @@ -205,13 +205,15 @@ func TestCloudAuditLogsSourceCheckImmutableFields(t *testing.T) { orig: &auditLogsSourceSpec, updated: CloudAuditLogsSourceSpec{ PubSubSpec: duckv1alpha1.PubSubSpec{ + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: "new-service-account", + }, Secret: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ Name: auditLogsSourceSpec.PubSubSpec.Secret.Name, }, Key: auditLogsSourceSpec.PubSubSpec.Secret.Key, }, - ServiceAccount: "new-service-account", SourceSpec: duckv1.SourceSpec{ Sink: auditLogsSourceSpec.PubSubSpec.Sink, }, diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go index 15504d3a2a..9599e769d8 100644 --- a/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go +++ b/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go @@ -47,11 +47,11 @@ type CloudPubSubSource struct { } var ( - _ kmeta.OwnerRefable = (*CloudSchedulerSource)(nil) - _ resourcesemantics.GenericCRD = (*CloudSchedulerSource)(nil) - _ kngcpduck.PubSubable = (*CloudSchedulerSource)(nil) - _ kngcpduck.Identifiable = (*CloudSchedulerSource)(nil) - _ = duck.VerifyType(&CloudSchedulerSource{}, &duckv1.Conditions{}) + _ kmeta.OwnerRefable = (*CloudPubSubSource)(nil) + _ resourcesemantics.GenericCRD = (*CloudPubSubSource)(nil) + _ kngcpduck.PubSubable = (*CloudPubSubSource)(nil) + _ kngcpduck.Identifiable = (*CloudPubSubSource)(nil) + _ = duck.VerifyType(&CloudPubSubSource{}, &duckv1.Conditions{}) ) // CloudPubSubSourceSpec defines the desired state of the CloudPubSubSource. @@ -129,7 +129,7 @@ var pubSubCondSet = apis.NewLivingConditionSet( // CloudPubSubSourceStatus defines the observed state of CloudPubSubSource. type CloudPubSubSourceStatus struct { // This brings in duck/v1beta1 Status as well as SinkURI - duckv1.SourceStatus `json:",inline"` + duckv1alpha1.PubSubStatus `json:",inline"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -146,11 +146,15 @@ func (s *CloudPubSubSource) GetGroupVersionKind() schema.GroupVersionKind { return SchemeGroupVersion.WithKind("CloudPubSubSource") } -// Methods for pubsubable interface +// Methods for identifiable interface. +// IdentitySpec returns the IdentitySpec portion of the Spec. +func (s *CloudPubSubSource) IdentitySpec() *duckv1alpha1.IdentitySpec { + return &s.Spec.IdentitySpec +} -// CloudPubSubSourceSpec returns the CloudPubSubSourceSpec portion of the Spec. -func (ps *CloudPubSubSource) PubSubSpec() *duckv1alpha1.PubSubSpec { - return &ps.Spec.PubSubSpec +// IdentityStatus returns the IdentityStatus portion of the Status. +func (s *CloudPubSubSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -158,7 +162,13 @@ func (ps *CloudPubSubSource) ConditionSet() *apis.ConditionSet { return &pubSubCondSet } -// Methods for identifiable interface -func (ps *CloudPubSubSource) GetIdentity() string { - return ps.Spec.ServiceAccount +// Methods for pubsubable interface. + +// CloudPubSubSourceSpec returns the CloudPubSubSourceSpec portion of the Spec. +func (ps *CloudPubSubSource) PubSubSpec() *duckv1alpha1.PubSubSpec { + return &ps.Spec.PubSubSpec +} + +func (s *CloudPubSubSource) PubSubStatus() *duckv1alpha1.PubSubStatus { + return &s.Status.PubSubStatus } diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go index 1d36b59b98..305009613e 100644 --- a/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go +++ b/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go @@ -17,6 +17,8 @@ limitations under the License. package v1alpha1 import ( + "github.com/google/go-cmp/cmp/cmpopts" + "knative.dev/pkg/apis" "testing" "time" @@ -92,17 +94,54 @@ func TestGetRetentionDuration_default(t *testing.T) { } } -func TestCloudPubSubSourceGetIdentity(t *testing.T) { +func TestCloudPubSubSourceIdentitySpec(t *testing.T) { s := &CloudPubSubSource{ Spec: CloudPubSubSourceSpec{ PubSubSpec: v1alpha1.PubSubSpec{ - ServiceAccount: "test@test", + IdentitySpec: v1alpha1.IdentitySpec{ + ServiceAccount: "test@test", + }, }, }, } want := "test@test" - got := s.GetIdentity() + got := s.IdentitySpec().ServiceAccount if diff := cmp.Diff(want, got); diff != "" { t.Errorf("failed to get expected (-want, +got) = %v", diff) } } + +func TestCloudPubSubSourceIdentityStatus(t *testing.T) { + s := &CloudPubSubSource{ + Status: CloudPubSubSourceStatus{ + PubSubStatus: v1alpha1.PubSubStatus{}, + }, + } + want := &v1alpha1.IdentityStatus{} + got := s.IdentityStatus() + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("failed to get expected (-want, +got) = %v", diff) + } +} + +func TestCloudPubSubSourceConditionSet(t *testing.T) { + want := []apis.Condition{{ + Type: v1alpha1.PullSubscriptionReady, + }, { + Type: apis.ConditionReady, + }} + c := &CloudPubSubSource{} + + c.ConditionSet().Manage(&c.Status).InitializeConditions() + var got []apis.Condition = c.Status.GetConditions() + + compareConditionTypes := cmp.Transformer("ConditionType", func(c apis.Condition) apis.ConditionType { + return c.Type + }) + sortConditionTypes := cmpopts.SortSlices(func(a, b apis.Condition) bool { + return a.Type < b.Type + }) + if diff := cmp.Diff(want, got, sortConditionTypes, compareConditionTypes); diff != "" { + t.Errorf("failed to get expected (-want, +got) = %v", diff) + } +} diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_validation_test.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_validation_test.go index 7c1389208d..6527501cad 100644 --- a/pkg/apis/events/v1alpha1/cloudpubsubsource_validation_test.go +++ b/pkg/apis/events/v1alpha1/cloudpubsubsource_validation_test.go @@ -291,13 +291,15 @@ func TestCloudPubSubSourceCheckImmutableFields(t *testing.T) { orig: &pubSubSourceSpec, updated: CloudPubSubSourceSpec{ PubSubSpec: duckv1alpha1.PubSubSpec{ + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: "new-service-account", + }, Secret: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ Name: pubSubSourceSpec.Secret.Name, }, Key: pubSubSourceSpec.Secret.Key, }, - ServiceAccount: "new-service-account", SourceSpec: duckv1.SourceSpec{ Sink: pubSubSourceSpec.Sink, }, diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle.go b/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle.go index fd2714b207..f0c68a7fac 100644 --- a/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle.go +++ b/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle.go @@ -18,8 +18,6 @@ package v1alpha1 import ( "knative.dev/pkg/apis" - - duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" ) // GetCondition returns the condition currently associated with the given type, or nil. @@ -42,42 +40,6 @@ func (s *CloudSchedulerSourceStatus) InitializeConditions() { schedulerCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionFailed sets the condition that the underlying PullSubscription -// is False and why. -func (s *CloudSchedulerSourceStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { - schedulerCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) -} - -// MarkPullSubscriptionUnknown sets the condition that the underlying PullSubscription -// is Unknown and why. -func (s *CloudSchedulerSourceStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { - schedulerCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) -} - -// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready. -func (s *CloudSchedulerSourceStatus) MarkPullSubscriptionReady() { - schedulerCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) -} - -// MarkTopicFailed sets the condition that the Topic was not created and why. -func (s *CloudSchedulerSourceStatus) MarkTopicFailed(reason, messageFormat string, messageA ...interface{}) { - schedulerCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) -} - -// MarkTopicUnknown sets the condition that the status of Topic is Unknown and why. -func (s *CloudSchedulerSourceStatus) MarkTopicUnknown(reason, messageFormat string, messageA ...interface{}) { - schedulerCondSet.Manage(s).MarkUnknown(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) -} - -// MarkTopicReady sets the condition that the underlying Topic was created -// successfully and sets the Status.TopicID to the specified topic -// and Status.ProjectID to the specified project. -func (s *CloudSchedulerSourceStatus) MarkTopicReady(topicID, projectID string) { - schedulerCondSet.Manage(s).MarkTrue(duckv1alpha1.TopicReady) - s.TopicID = topicID - s.ProjectID = projectID -} - // MarkJobNotReady sets the condition that the CloudSchedulerSource Job has not been // successfully created. func (s *CloudSchedulerSourceStatus) MarkJobNotReady(reason, messageFormat string, messageA ...interface{}) { diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle_test.go b/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle_test.go index a2475659be..85b30b5f5a 100644 --- a/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/cloudschedulersource_lifecycle_test.go @@ -39,33 +39,33 @@ func TestCloudSchedulerSourceStatusIsReady(t *testing.T) { }, { name: "initialized", s: func() *CloudSchedulerSourceStatus { - s := &CloudSchedulerSourceStatus{} - s.InitializeConditions() - return s + s := &CloudSchedulerSource{} + s.Status.InitializeConditions() + return &s.Status }(), wantConditionStatus: corev1.ConditionUnknown, want: false, }, { name: "the status of topic is false", s: func() *CloudSchedulerSourceStatus { - s := &CloudSchedulerSourceStatus{} - s.InitializeConditions() - s.MarkPullSubscriptionReady() - s.MarkJobReady("jobName") - s.MarkTopicFailed("TopicFailed", "the status of topic is false") - return s + s := &CloudSchedulerSource{} + s.Status.InitializeConditions() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkJobReady("jobName") + s.Status.MarkTopicFailed(s.ConditionSet(), "TopicFailed", "the status of topic is false") + return &s.Status }(), wantConditionStatus: corev1.ConditionFalse, want: false, }, { name: "the status of topic is unknown", s: func() *CloudSchedulerSourceStatus { - s := &CloudSchedulerSourceStatus{} - s.InitializeConditions() - s.MarkPullSubscriptionReady() - s.MarkJobReady("jobName") - s.MarkTopicUnknown("TopicUnknown", "the status of topic is unknown") - return s + s := &CloudSchedulerSource{} + s.Status.InitializeConditions() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkJobReady("jobName") + s.Status.MarkTopicUnknown(s.ConditionSet(), "TopicUnknown", "the status of topic is unknown") + return &s.Status }(), wantConditionStatus: corev1.ConditionUnknown, want: false, @@ -73,24 +73,24 @@ func TestCloudSchedulerSourceStatusIsReady(t *testing.T) { { name: "the status pullsubscription is false", s: func() *CloudSchedulerSourceStatus { - s := &CloudSchedulerSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady("topicID", "projectID") - s.MarkPullSubscriptionFailed("PullSubscriptionFailed", "the status of pullsubscription is false") - s.MarkJobReady("jobName") - return s + s := &CloudSchedulerSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionFailed(s.ConditionSet(), "PullSubscriptionFailed", "the status of pullsubscription is false") + s.Status.MarkJobReady("jobName") + return &s.Status }(), wantConditionStatus: corev1.ConditionFalse, want: false, }, { name: "the status pullsubscription is unknown", s: func() *CloudSchedulerSourceStatus { - s := &CloudSchedulerSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady("topicID", "projectID") - s.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "the status of pullsubscription is unknown") - s.MarkJobReady("jobName") - return s + s := &CloudSchedulerSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionUnknown(s.ConditionSet(), "PullSubscriptionUnknown", "the status of pullsubscription is unknown") + s.Status.MarkJobReady("jobName") + return &s.Status }(), wantConditionStatus: corev1.ConditionUnknown, want: false, @@ -98,22 +98,22 @@ func TestCloudSchedulerSourceStatusIsReady(t *testing.T) { { name: "job not ready", s: func() *CloudSchedulerSourceStatus { - s := &CloudSchedulerSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady("topicID", "projectID") - s.MarkPullSubscriptionReady() - s.MarkJobNotReady("NotReady", "ps not ready") - return s + s := &CloudSchedulerSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkJobNotReady("NotReady", "ps not ready") + return &s.Status }(), }, { name: "ready", s: func() *CloudSchedulerSourceStatus { - s := &CloudSchedulerSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady("topicID", "projectID") - s.MarkPullSubscriptionReady() - s.MarkJobReady("jobName") - return s + s := &CloudSchedulerSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkJobReady("jobName") + return &s.Status }(), want: true, }} diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_types.go b/pkg/apis/events/v1alpha1/cloudschedulersource_types.go index 6f2c22fc9d..cd8681f210 100644 --- a/pkg/apis/events/v1alpha1/cloudschedulersource_types.go +++ b/pkg/apis/events/v1alpha1/cloudschedulersource_types.go @@ -110,15 +110,15 @@ func (scheduler *CloudSchedulerSource) GetGroupVersionKind() schema.GroupVersion return SchemeGroupVersion.WithKind("CloudSchedulerSource") } -// Methods for pubsubable interface -// PubSubSpec returns the PubSubSpec portion of the Spec. -func (s *CloudSchedulerSource) PubSubSpec() *duckv1alpha1.PubSubSpec { - return &s.Spec.PubSubSpec +// Methods for identifiable interface +// IdentitySpec returns the IdentitySpec portion of the Spec. +func (s *CloudSchedulerSource) IdentitySpec() *duckv1alpha1.IdentitySpec { + return &s.Spec.IdentitySpec } -// PubSubStatus returns the PubSubStatus portion of the Status. -func (s *CloudSchedulerSource) PubSubStatus() *duckv1alpha1.PubSubStatus { - return &s.Status.PubSubStatus +// IdentityStatus returns the IdentityStatus portion of the Status. +func (s *CloudSchedulerSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -126,9 +126,15 @@ func (s *CloudSchedulerSource) ConditionSet() *apis.ConditionSet { return &schedulerCondSet } -// Methods for identifiable interface -func (s *CloudSchedulerSource) GetIdentity() string { - return s.Spec.ServiceAccount +// Methods for pubsubable interface +// PubSubSpec returns the PubSubSpec portion of the Spec. +func (s *CloudSchedulerSource) PubSubSpec() *duckv1alpha1.PubSubSpec { + return &s.Spec.PubSubSpec +} + +// PubSubStatus returns the PubSubStatus portion of the Status. +func (s *CloudSchedulerSource) PubSubStatus() *duckv1alpha1.PubSubStatus { + return &s.Status.PubSubStatus } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_types_test.go b/pkg/apis/events/v1alpha1/cloudschedulersource_types_test.go index 0cc26b7847..d4ae1f0621 100644 --- a/pkg/apis/events/v1alpha1/cloudschedulersource_types_test.go +++ b/pkg/apis/events/v1alpha1/cloudschedulersource_types_test.go @@ -78,16 +78,31 @@ func TestCloudSchedulerSourceConditionSet(t *testing.T) { } } -func TestCloudSchedulerSourceGetIdentity(t *testing.T) { +func TestCloudSchedulerSourceIdentitySpec(t *testing.T) { s := &CloudSchedulerSource{ Spec: CloudSchedulerSourceSpec{ PubSubSpec: v1alpha1.PubSubSpec{ - ServiceAccount: "test@test", + IdentitySpec: v1alpha1.IdentitySpec{ + ServiceAccount: "test@test", + }, }, }, } want := "test@test" - got := s.GetIdentity() + got := s.IdentitySpec().ServiceAccount + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("failed to get expected (-want, +got) = %v", diff) + } +} + +func TestCloudSchedulerSourceIdentityStatus(t *testing.T) { + s := &CloudSchedulerSource{ + Status: CloudSchedulerSourceStatus{ + PubSubStatus: v1alpha1.PubSubStatus{}, + }, + } + want := &v1alpha1.IdentityStatus{} + got := s.IdentityStatus() 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_test.go b/pkg/apis/events/v1alpha1/cloudschedulersource_validation_test.go index 693d908d4b..05b7288866 100644 --- a/pkg/apis/events/v1alpha1/cloudschedulersource_validation_test.go +++ b/pkg/apis/events/v1alpha1/cloudschedulersource_validation_test.go @@ -280,6 +280,9 @@ func TestCloudSchedulerSourceSpecValidationFields(t *testing.T) { Schedule: "* * * * *", Data: "data", PubSubSpec: duckv1alpha1.PubSubSpec{ + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: invalidServiceAccountName, + }, SourceSpec: duckv1.SourceSpec{ Sink: duckv1.Destination{ Ref: &duckv1.KReference{ @@ -290,7 +293,6 @@ func TestCloudSchedulerSourceSpecValidationFields(t *testing.T) { }, }, }, - ServiceAccount: invalidServiceAccountName, }, }, want: func() *apis.FieldError { @@ -307,6 +309,9 @@ func TestCloudSchedulerSourceSpecValidationFields(t *testing.T) { Schedule: "* * * * *", Data: "data", PubSubSpec: duckv1alpha1.PubSubSpec{ + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: invalidServiceAccountName, + }, SourceSpec: duckv1.SourceSpec{ Sink: duckv1.Destination{ Ref: &duckv1.KReference{ @@ -317,7 +322,6 @@ func TestCloudSchedulerSourceSpecValidationFields(t *testing.T) { }, }, }, - ServiceAccount: invalidServiceAccountName, Secret: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{}, Key: "secret-test-key", @@ -423,9 +427,11 @@ func TestCloudSchedulerSourceSpecCheckImmutableFields(t *testing.T) { Schedule: schedulerWithSecret.Schedule, Data: schedulerWithSecret.Data, PubSubSpec: duckv1alpha1.PubSubSpec{ - SourceSpec: schedulerWithSecret.SourceSpec, - Secret: schedulerWithSecret.Secret, - ServiceAccount: "new-service-account", + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: "new-service-account", + }, + SourceSpec: schedulerWithSecret.SourceSpec, + Secret: schedulerWithSecret.Secret, }, }, allowed: false, diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle.go b/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle.go index d4b5f3a5d9..d7112d2bbf 100644 --- a/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle.go +++ b/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle.go @@ -18,8 +18,6 @@ package v1alpha1 import ( "knative.dev/pkg/apis" - - duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" ) // GetCondition returns the condition currently associated with the given type, or nil. @@ -42,38 +40,6 @@ func (s *CloudStorageSourceStatus) InitializeConditions() { storageCondSet.Manage(s).InitializeConditions() } -// MarkPullSubscriptionFailed sets the condition that the status of underlying PullSubscription -// is False and why. -func (s *CloudStorageSourceStatus) MarkPullSubscriptionFailed(reason, messageFormat string, messageA ...interface{}) { - storageCondSet.Manage(s).MarkFalse(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) -} - -// MarkPullSubscriptionUnknown sets the condition that the status of underlying PullSubscription -// is Unknown and why. -func (s *CloudStorageSourceStatus) MarkPullSubscriptionUnknown(reason, messageFormat string, messageA ...interface{}) { - storageCondSet.Manage(s).MarkUnknown(duckv1alpha1.PullSubscriptionReady, reason, messageFormat, messageA...) -} - -// MarkPullSubscriptionReady sets the condition that the underlying PullSubscription is ready. -func (s *CloudStorageSourceStatus) MarkPullSubscriptionReady() { - storageCondSet.Manage(s).MarkTrue(duckv1alpha1.PullSubscriptionReady) -} - -// MarkTopicFailed sets the condition that the status of PubSub topic is False why. -func (s *CloudStorageSourceStatus) MarkTopicFailed(reason, messageFormat string, messageA ...interface{}) { - storageCondSet.Manage(s).MarkFalse(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) -} - -// MarkTopicUnknown sets the condition that the status of PubSub topic is Unknown why. -func (s *CloudStorageSourceStatus) MarkTopicUnknown(reason, messageFormat string, messageA ...interface{}) { - storageCondSet.Manage(s).MarkUnknown(duckv1alpha1.TopicReady, reason, messageFormat, messageA...) -} - -// MarkTopicReady sets the condition that the underlying PubSub topic was created successfully. -func (s *CloudStorageSourceStatus) MarkTopicReady() { - storageCondSet.Manage(s).MarkTrue(duckv1alpha1.TopicReady) -} - // MarkNotificationNotReady sets the condition that the GCS has not been configured // to send Notifications and why. func (s *CloudStorageSourceStatus) MarkNotificationNotReady(reason, messageFormat string, messageA ...interface{}) { diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle_test.go b/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle_test.go index bc773cd680..578a75d31e 100644 --- a/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle_test.go +++ b/pkg/apis/events/v1alpha1/cloudstoragesource_lifecycle_test.go @@ -48,36 +48,36 @@ func TestCloudStorageSourceStatusIsReady(t *testing.T) { }, { name: "the status of topic is false", s: func() *CloudStorageSourceStatus { - s := &CloudStorageSourceStatus{} - s.InitializeConditions() - s.MarkPullSubscriptionReady() - s.MarkNotificationReady("notificationID") - s.MarkTopicFailed("TopicFailed", "the status of topic is false") - return s + s := &CloudStorageSource{} + s.Status.InitializeConditions() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkNotificationReady("notificationID") + s.Status.MarkTopicFailed(s.ConditionSet(), "TopicFailed", "the status of topic is false") + return &s.Status }(), wantConditionStatus: corev1.ConditionFalse, want: false, }, { name: "the status of topic is unknown", s: func() *CloudStorageSourceStatus { - s := &CloudStorageSourceStatus{} - s.InitializeConditions() - s.MarkPullSubscriptionReady() - s.MarkNotificationReady("notificationID") - s.MarkTopicUnknown("TopicUnknown", "the status of topic is unknown") - return s + s := &CloudStorageSource{} + s.Status.InitializeConditions() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkNotificationReady("notificationID") + s.Status.MarkTopicUnknown(s.ConditionSet(), "TopicUnknown", "the status of topic is unknown") + return &s.Status }(), wantConditionStatus: corev1.ConditionUnknown, want: false, }, { name: "the status of pullsubscription is false", s: func() *CloudStorageSourceStatus { - s := &CloudStorageSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionFailed("PullSubscriptionFailed", "the status of pullsubscription is false") - s.MarkNotificationReady("notificationID") - return s + s := &CloudStorageSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionFailed(s.ConditionSet(), "PullSubscriptionFailed", "the status of pullsubscription is false") + s.Status.MarkNotificationReady("notificationID") + return &s.Status }(), wantConditionStatus: corev1.ConditionFalse, want: false, @@ -85,34 +85,34 @@ func TestCloudStorageSourceStatusIsReady(t *testing.T) { { name: "the status of pullsubscription is unknown", s: func() *CloudStorageSourceStatus { - s := &CloudStorageSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionUnknown("PullSubscriptionUnknown", "the status of pullsubscription is unknown") - s.MarkNotificationReady("notificationID") - return s + s := &CloudStorageSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionUnknown(s.ConditionSet(), "PullSubscriptionUnknown", "the status of pullsubscription is unknown") + s.Status.MarkNotificationReady("notificationID") + return &s.Status }(), wantConditionStatus: corev1.ConditionUnknown, want: false, }, { name: "notification not ready", s: func() *CloudStorageSourceStatus { - s := &CloudStorageSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionReady() - s.MarkNotificationNotReady("NotReady", "notification not ready") - return s + s := &CloudStorageSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkNotificationNotReady("NotReady", "notification not ready") + return &s.Status }(), }, { name: "ready", s: func() *CloudStorageSourceStatus { - s := &CloudStorageSourceStatus{} - s.InitializeConditions() - s.MarkTopicReady() - s.MarkPullSubscriptionReady() - s.MarkNotificationReady("notificationID") - return s + s := &CloudStorageSource{} + s.Status.InitializeConditions() + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) + s.Status.MarkNotificationReady("notificationID") + return &s.Status }(), wantConditionStatus: corev1.ConditionTrue, want: true, diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_types.go b/pkg/apis/events/v1alpha1/cloudstoragesource_types.go index 206a60a628..010a5fa15f 100644 --- a/pkg/apis/events/v1alpha1/cloudstoragesource_types.go +++ b/pkg/apis/events/v1alpha1/cloudstoragesource_types.go @@ -126,16 +126,15 @@ func (storage *CloudStorageSource) GetGroupVersionKind() schema.GroupVersionKind return SchemeGroupVersion.WithKind("CloudStorageSource") } -// Methods for pubsubable interface - -// PubSubSpec returns the PubSubSpec portion of the Spec. -func (s *CloudStorageSource) PubSubSpec() *duckv1alpha1.PubSubSpec { - return &s.Spec.PubSubSpec +// Methods for identifiable interface. +// IdentitySpec returns the IdentitySpec portion of the Spec. +func (s *CloudStorageSource) IdentitySpec() *duckv1alpha1.IdentitySpec { + return &s.Spec.IdentitySpec } -// PubSubStatus returns the PubSubStatus portion of the Status. -func (s *CloudStorageSource) PubSubStatus() *duckv1alpha1.PubSubStatus { - return &s.Status.PubSubStatus +// IdentityStatus returns the IdentityStatus portion of the Status. +func (s *CloudStorageSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -143,9 +142,16 @@ func (s *CloudStorageSource) ConditionSet() *apis.ConditionSet { return &storageCondSet } -// Methods for identifiable interface -func (s *CloudStorageSource) GetIdentity() string { - return s.Spec.ServiceAccount +// Methods for pubsubable interface. + +// PubSubSpec returns the PubSubSpec portion of the Spec. +func (s *CloudStorageSource) PubSubSpec() *duckv1alpha1.PubSubSpec { + return &s.Spec.PubSubSpec +} + +// PubSubStatus returns the PubSubStatus portion of the Status. +func (s *CloudStorageSource) PubSubStatus() *duckv1alpha1.PubSubStatus { + return &s.Status.PubSubStatus } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_types_test.go b/pkg/apis/events/v1alpha1/cloudstoragesource_types_test.go index 999743841b..d8174247df 100644 --- a/pkg/apis/events/v1alpha1/cloudstoragesource_types_test.go +++ b/pkg/apis/events/v1alpha1/cloudstoragesource_types_test.go @@ -76,16 +76,31 @@ func TestCloudStorageSourceSourceConditionSet(t *testing.T) { } } -func TestCloudStorageSourceGetIdentity(t *testing.T) { +func TestCloudStorageSourceIdentitySpec(t *testing.T) { s := &CloudStorageSource{ Spec: CloudStorageSourceSpec{ PubSubSpec: v1alpha1.PubSubSpec{ - ServiceAccount: "test@test", + IdentitySpec: v1alpha1.IdentitySpec{ + ServiceAccount: "test@test", + }, }, }, } want := "test@test" - got := s.GetIdentity() + got := s.IdentitySpec().ServiceAccount + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("failed to get expected (-want, +got) = %v", diff) + } +} + +func TestCloudStorageSourceIdentityStatus(t *testing.T) { + s := &CloudStorageSource{ + Status: CloudStorageSourceStatus{ + PubSubStatus: v1alpha1.PubSubStatus{}, + }, + } + want := &v1alpha1.IdentityStatus{} + got := s.IdentityStatus() 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_test.go b/pkg/apis/events/v1alpha1/cloudstoragesource_validation_test.go index a9dfcca29b..5bd0016162 100644 --- a/pkg/apis/events/v1alpha1/cloudstoragesource_validation_test.go +++ b/pkg/apis/events/v1alpha1/cloudstoragesource_validation_test.go @@ -256,6 +256,9 @@ func TestSpecValidationFields(t *testing.T) { spec: &CloudStorageSourceSpec{ Bucket: "my-test-bucket", PubSubSpec: duckv1alpha1.PubSubSpec{ + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: invalidServiceAccountName, + }, SourceSpec: duckv1.SourceSpec{ Sink: duckv1.Destination{ Ref: &duckv1.KReference{ @@ -266,7 +269,6 @@ func TestSpecValidationFields(t *testing.T) { }, }, }, - ServiceAccount: invalidServiceAccountName, }, }, want: func() *apis.FieldError { @@ -281,6 +283,9 @@ func TestSpecValidationFields(t *testing.T) { spec: &CloudStorageSourceSpec{ Bucket: "my-test-bucket", PubSubSpec: duckv1alpha1.PubSubSpec{ + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: validServiceAccountName, + }, SourceSpec: duckv1.SourceSpec{ Sink: duckv1.Destination{ Ref: &duckv1.KReference{ @@ -291,7 +296,6 @@ func TestSpecValidationFields(t *testing.T) { }, }, }, - ServiceAccount: validServiceAccountName, }, }, want: nil, @@ -300,6 +304,9 @@ func TestSpecValidationFields(t *testing.T) { spec: &CloudStorageSourceSpec{ Bucket: "my-test-bucket", PubSubSpec: duckv1alpha1.PubSubSpec{ + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: invalidServiceAccountName, + }, SourceSpec: duckv1.SourceSpec{ Sink: duckv1.Destination{ Ref: &duckv1.KReference{ @@ -310,7 +317,6 @@ func TestSpecValidationFields(t *testing.T) { }, }, }, - ServiceAccount: invalidServiceAccountName, Secret: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{}, Key: "secret-test-key", @@ -432,9 +438,11 @@ func TestCheckImmutableFields(t *testing.T) { ObjectNamePrefix: storageSourceSpec.ObjectNamePrefix, PayloadFormat: storageSourceSpec.PayloadFormat, PubSubSpec: duckv1alpha1.PubSubSpec{ - SourceSpec: storageSourceSpec.SourceSpec, - Secret: storageSourceSpec.Secret, - ServiceAccount: "new-service-account", + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: "new-service-account", + }, + SourceSpec: storageSourceSpec.SourceSpec, + Secret: storageSourceSpec.Secret, }, }, allowed: false, diff --git a/pkg/apis/events/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/events/v1alpha1/zz_generated.deepcopy.go index eee62f7553..3c3ed1f199 100644 --- a/pkg/apis/events/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/events/v1alpha1/zz_generated.deepcopy.go @@ -210,7 +210,7 @@ func (in *CloudPubSubSourceSpec) DeepCopy() *CloudPubSubSourceSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CloudPubSubSourceStatus) DeepCopyInto(out *CloudPubSubSourceStatus) { *out = *in - in.SourceStatus.DeepCopyInto(&out.SourceStatus) + in.PubSubStatus.DeepCopyInto(&out.PubSubStatus) return } diff --git a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go index 7f29566702..418c4e2a3e 100644 --- a/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_lifecycle_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" "github.com/google/knative-gcp/pkg/apis/pubsub/v1alpha1" corev1 "k8s.io/api/core/v1" "knative.dev/pkg/apis" @@ -46,9 +47,11 @@ func TestChannelGetCondition(t *testing.T) { }{{ name: "single condition", cs: &ChannelStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{ - condReady, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{ + condReady, + }, }, }, }, @@ -74,65 +77,75 @@ func TestChannelInitializeConditions(t *testing.T) { name: "empty", cs: &ChannelStatus{}, want: &ChannelStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: ChannelConditionAddressable, - Status: corev1.ConditionUnknown, - }, { - Type: ChannelConditionReady, - Status: corev1.ConditionUnknown, - }, { - Type: ChannelConditionTopicReady, - Status: corev1.ConditionUnknown, - }}, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{{ + Type: ChannelConditionAddressable, + Status: corev1.ConditionUnknown, + }, { + Type: ChannelConditionReady, + Status: corev1.ConditionUnknown, + }, { + Type: ChannelConditionTopicReady, + Status: corev1.ConditionUnknown, + }}, + }, }, }, }, { name: "one false", cs: &ChannelStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: ChannelConditionAddressable, - Status: corev1.ConditionFalse, - }}, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{{ + Type: ChannelConditionAddressable, + Status: corev1.ConditionFalse, + }}, + }, }, }, want: &ChannelStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: ChannelConditionAddressable, - Status: corev1.ConditionFalse, - }, { - Type: ChannelConditionReady, - Status: corev1.ConditionUnknown, - }, { - Type: ChannelConditionTopicReady, - Status: corev1.ConditionUnknown, - }}, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{{ + Type: ChannelConditionAddressable, + Status: corev1.ConditionFalse, + }, { + Type: ChannelConditionReady, + Status: corev1.ConditionUnknown, + }, { + Type: ChannelConditionTopicReady, + Status: corev1.ConditionUnknown, + }}, + }, }, }, }, { name: "one true", cs: &ChannelStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: ChannelConditionAddressable, - Status: corev1.ConditionTrue, - }}, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{{ + Type: ChannelConditionAddressable, + Status: corev1.ConditionTrue, + }}, + }, }, }, want: &ChannelStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: ChannelConditionAddressable, - Status: corev1.ConditionTrue, - }, { - Type: ChannelConditionReady, - Status: corev1.ConditionUnknown, - }, { - Type: ChannelConditionTopicReady, - Status: corev1.ConditionUnknown, - }}, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{{ + Type: ChannelConditionAddressable, + Status: corev1.ConditionTrue, + }, { + Type: ChannelConditionReady, + Status: corev1.ConditionUnknown, + }, { + Type: ChannelConditionTopicReady, + Status: corev1.ConditionUnknown, + }}, + }, }, }, }} @@ -207,17 +220,19 @@ func TestPubSubChannelStatus_SetAddressable(t *testing.T) { }{ "empty string": { want: &ChannelStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{ - { - Type: ChannelConditionAddressable, - Status: corev1.ConditionFalse, - }, - // Note that Ready is here because when the condition is marked False, duck - // automatically sets Ready to false. - { - Type: ChannelConditionReady, - Status: corev1.ConditionFalse, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{ + { + Type: ChannelConditionAddressable, + Status: corev1.ConditionFalse, + }, + // Note that Ready is here because when the condition is marked False, duck + // automatically sets Ready to false. + { + Type: ChannelConditionReady, + Status: corev1.ConditionFalse, + }, }, }, }, @@ -235,15 +250,17 @@ func TestPubSubChannelStatus_SetAddressable(t *testing.T) { }, }, }, - Status: duckv1.Status{ - Conditions: []apis.Condition{ - { - Type: ChannelConditionAddressable, - Status: corev1.ConditionTrue, - }, - { - Type: ChannelConditionReady, - Status: corev1.ConditionUnknown, + IdentityStatus: duckv1alpha1.IdentityStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{ + { + Type: ChannelConditionAddressable, + Status: corev1.ConditionTrue, + }, + { + Type: ChannelConditionReady, + Status: corev1.ConditionUnknown, + }, }, }, }, diff --git a/pkg/apis/messaging/v1alpha1/channel_types.go b/pkg/apis/messaging/v1alpha1/channel_types.go index 54f69839ef..afc60d3c8a 100644 --- a/pkg/apis/messaging/v1alpha1/channel_types.go +++ b/pkg/apis/messaging/v1alpha1/channel_types.go @@ -26,6 +26,7 @@ import ( duckv1 "knative.dev/pkg/apis/duck/v1" "knative.dev/pkg/webhook/resourcesemantics" + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" "github.com/google/knative-gcp/pkg/duck" ) @@ -61,11 +62,7 @@ var ( // receiving events from this Channel. // arguments for a Channel. type ChannelSpec struct { - // ServiceAccount 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 - ServiceAccount string `json:"serviceAccount,omitempty"` - + duckv1alpha1.IdentitySpec `json:",inline"` // Secret is the credential to use to create, publish, and poll the Pub/Sub // Topic and Subscriptions. The value of the secret entry must be a // service account key in the JSON format @@ -104,10 +101,7 @@ const ( // ChannelStatus represents the current state of a Channel. type ChannelStatus struct { - // inherits duck/v1beta1 Status, which currently provides: - // * ObservedGeneration - the 'Generation' of the Service that was last processed by the controller. - // * Conditions - the latest available observations of a resource's current state. - duckv1.Status `json:",inline"` + duckv1alpha1.IdentityStatus `json:",inline"` // Channel is Addressable. It currently exposes the endpoint as a // fully-qualified DNS name which will distribute traffic over the @@ -128,9 +122,20 @@ type ChannelStatus struct { TopicID string `json:"topicId,omitempty"` } -// Methods for identifiable interface -func (c *Channel) GetIdentity() string { - return c.Spec.ServiceAccount +// Methods for identifiable interface. +// IdentitySpec returns the IdentitySpec portion of the Spec. +func (c *Channel) IdentitySpec() *duckv1alpha1.IdentitySpec { + return &c.Spec.IdentitySpec +} + +// IdentityStatus returns the IdentityStatus portion of the Status. +func (c *Channel) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &c.Status.IdentityStatus +} + +// ConditionSet returns the apis.ConditionSet of the embedding object +func (s *Channel) ConditionSet() *apis.ConditionSet { + return &channelCondSet } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/messaging/v1alpha1/channel_types_test.go b/pkg/apis/messaging/v1alpha1/channel_types_test.go index f80ce0e655..df006a8616 100644 --- a/pkg/apis/messaging/v1alpha1/channel_types_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_types_test.go @@ -17,11 +17,14 @@ limitations under the License. package v1alpha1 import ( + "github.com/google/go-cmp/cmp/cmpopts" + "knative.dev/pkg/apis" "testing" "k8s.io/apimachinery/pkg/runtime/schema" "github.com/google/go-cmp/cmp" + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" ) func TestChannelGetGroupVersionKind(t *testing.T) { @@ -39,15 +42,54 @@ func TestChannelGetGroupVersionKind(t *testing.T) { } } -func TestChannelGetIdentity(t *testing.T) { +func TestChannelIdentitySpec(t *testing.T) { s := &Channel{ Spec: ChannelSpec{ - ServiceAccount: "test@test", + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: "test@test", + }, }, } want := "test@test" - got := s.GetIdentity() + got := s.IdentitySpec().ServiceAccount if diff := cmp.Diff(want, got); diff != "" { t.Errorf("failed to get expected (-want, +got) = %v", diff) } } + +func TestChannelIdentityStatus(t *testing.T) { + s := &Channel{ + Status: ChannelStatus{ + IdentityStatus: duckv1alpha1.IdentityStatus{}, + }, + } + want := &duckv1alpha1.IdentityStatus{} + got := s.IdentityStatus() + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("failed to get expected (-want, +got) = %v", diff) + } +} + +func TestChannelConditionSet(t *testing.T) { + want := []apis.Condition{{ + Type: ChannelConditionAddressable, + }, { + Type: ChannelConditionTopicReady, + }, { + Type: apis.ConditionReady, + }} + c := &Channel{} + + c.ConditionSet().Manage(&c.Status).InitializeConditions() + var got []apis.Condition = c.Status.GetConditions() + + compareConditionTypes := cmp.Transformer("ConditionType", func(c apis.Condition) apis.ConditionType { + return c.Type + }) + sortConditionTypes := cmpopts.SortSlices(func(a, b apis.Condition) bool { + return a.Type < b.Type + }) + if diff := cmp.Diff(want, got, sortConditionTypes, compareConditionTypes); diff != "" { + t.Errorf("failed to get expected (-want, +got) = %v", diff) + } +} diff --git a/pkg/apis/messaging/v1alpha1/channel_validation_test.go b/pkg/apis/messaging/v1alpha1/channel_validation_test.go index 68421f5626..8779537a1e 100644 --- a/pkg/apis/messaging/v1alpha1/channel_validation_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_validation_test.go @@ -21,6 +21,8 @@ import ( "testing" "github.com/google/go-cmp/cmp" + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" + eventingduck "knative.dev/eventing/pkg/apis/duck/v1alpha1" "knative.dev/pkg/apis" "knative.dev/pkg/webhook/resourcesemantics" @@ -114,7 +116,9 @@ func TestChannelValidation(t *testing.T) { name: "invalid GCP service account", cr: &Channel{ Spec: ChannelSpec{ - ServiceAccount: invalidServiceAccountName, + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: invalidServiceAccountName, + }, Subscribable: &eventingduck.Subscribable{ Subscribers: []eventingduck.SubscriberSpec{{ SubscriberURI: apis.HTTP("subscriberendpoint"), @@ -133,7 +137,9 @@ func TestChannelValidation(t *testing.T) { name: "valid GCP service account", cr: &Channel{ Spec: ChannelSpec{ - ServiceAccount: validServiceAccountName, + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: validServiceAccountName, + }, Subscribable: &eventingduck.Subscribable{ Subscribers: []eventingduck.SubscriberSpec{{ SubscriberURI: apis.HTTP("subscriberendpoint"), @@ -146,8 +152,10 @@ func TestChannelValidation(t *testing.T) { name: "have GCP service account and secret at the same time", cr: &Channel{ Spec: ChannelSpec{ - ServiceAccount: validServiceAccountName, - Secret: defaultSecretSelector(), + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: validServiceAccountName, + }, + Secret: defaultSecretSelector(), Subscribable: &eventingduck.Subscribable{ Subscribers: []eventingduck.SubscriberSpec{{ SubscriberURI: apis.HTTP("subscriberendpoint"), @@ -186,7 +194,9 @@ func TestCheckImmutableFields(t *testing.T) { "ServiceAccount changed": { orig: &channelSpec, updated: ChannelSpec{ - ServiceAccount: "new-service-account", + IdentitySpec: duckv1alpha1.IdentitySpec{ + ServiceAccount: "new-service-account", + }, }, allowed: false, }, diff --git a/pkg/apis/messaging/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/messaging/v1alpha1/zz_generated.deepcopy.go index 3c542fc7b9..d4b21d59bf 100644 --- a/pkg/apis/messaging/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/messaging/v1alpha1/zz_generated.deepcopy.go @@ -90,6 +90,7 @@ func (in *ChannelList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ChannelSpec) DeepCopyInto(out *ChannelSpec) { *out = *in + out.IdentitySpec = in.IdentitySpec if in.Secret != nil { in, out := &in.Secret, &out.Secret *out = new(v1.SecretKeySelector) @@ -116,7 +117,7 @@ func (in *ChannelSpec) DeepCopy() *ChannelSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ChannelStatus) DeepCopyInto(out *ChannelStatus) { *out = *in - in.Status.DeepCopyInto(&out.Status) + in.IdentityStatus.DeepCopyInto(&out.IdentityStatus) in.AddressStatus.DeepCopyInto(&out.AddressStatus) in.SubscribableTypeStatus.DeepCopyInto(&out.SubscribableTypeStatus) return diff --git a/pkg/duck/identifiable.go b/pkg/duck/identifiable.go index 6ee7af556b..371fc9ae08 100644 --- a/pkg/duck/identifiable.go +++ b/pkg/duck/identifiable.go @@ -16,10 +16,19 @@ limitations under the License. package duck -import "knative.dev/pkg/kmeta" +import ( + "knative.dev/pkg/apis" + "knative.dev/pkg/kmeta" + + duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" +) type Identifiable interface { kmeta.OwnerRefable - // GetIdentity returns identifiable's identity. - GetIdentity() string + // IdentitySpec returns the IdentitySpec portion of the Spec. + IdentitySpec() *duckv1alpha1.IdentitySpec + // IdentityStatus returns the IdentityStatus portion of the Status. + IdentityStatus() *duckv1alpha1.IdentityStatus + // ConditionSet returns the apis.ConditionSet of the embedding object + ConditionSet() *apis.ConditionSet } diff --git a/pkg/duck/pubsubable.go b/pkg/duck/pubsubable.go index 449bf6e42d..0e91d14a7d 100644 --- a/pkg/duck/pubsubable.go +++ b/pkg/duck/pubsubable.go @@ -18,23 +18,14 @@ package duck import ( duckv1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" - - "knative.dev/pkg/apis" - "knative.dev/pkg/kmeta" ) // PubSubable is an interface that each duckv1alpha1.PubSub duck type must // support in order to get reconciled properly in a generic way. type PubSubable interface { - kmeta.OwnerRefable + Identifiable // PubSubSpec returns the PubSubSpec portion of the Spec. PubSubSpec() *duckv1alpha1.PubSubSpec // PubSubStatus returns the PubSubStatus portion of the Status. PubSubStatus() *duckv1alpha1.PubSubStatus - // ConditionSet returns the apis.ConditionSet of the embedding object - // This Set must have the following Conditions defined in it. - // "TopicReady", - // "PullSubscriptionReady", - // Which will be set appropriately automagically by the pubsub_reconciler.go - ConditionSet() *apis.ConditionSet } diff --git a/pkg/reconciler/events/auditlogs/auditlogs_test.go b/pkg/reconciler/events/auditlogs/auditlogs_test.go index 2ad33d7aef..1535d3c30c 100644 --- a/pkg/reconciler/events/auditlogs/auditlogs_test.go +++ b/pkg/reconciler/events/auditlogs/auditlogs_test.go @@ -95,7 +95,7 @@ var ( Key: "key.json", } - gServiceAccount = "test@test" + gServiceAccount = "test123@test123.iam.gserviceaccount.com" sinkURI = apis.HTTP(sinkDNS) ) @@ -124,6 +124,7 @@ func patchFinalizers(namespace, name string, add bool) clientgotesting.PatchActi return action } +// TODO add a unit test for successfully creating a k8s service account, after issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { calSinkURL := sinkURI @@ -134,49 +135,6 @@ func TestAllCases(t *testing.T) { Name: "key not found", // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", - }, { - Name: "k8s service account created", - Objects: []runtime.Object{ - NewCloudAuditLogsSource(sourceName, testNS, - WithCloudAuditLogsSourceSink(sinkGVK, sinkName), - WithCloudAuditLogsSourceProjectID(testProject), - WithCloudAuditLogsSourceMethodName(testMethodName), - WithCloudAuditLogsSourceServiceName(testServiceName), - WithCloudAuditLogsSourceGCPServiceAccount(gServiceAccount)), - }, - Key: testNS + "/" + sourceName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudAuditLogsSource(sourceName, testNS, - WithInitCloudAuditLogsSourceConditions, - WithCloudAuditLogsSourceSink(sinkGVK, sinkName), - WithCloudAuditLogsSourceProjectID(testProject), - WithCloudAuditLogsSourceMethodName(testMethodName), - WithCloudAuditLogsSourceServiceName(testServiceName), - WithCloudAuditLogsSourceGCPServiceAccount(gServiceAccount)), - }}, - WantCreates: []runtime.Object{ - NewServiceAccount("test", testNS, gServiceAccount), - }, - WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewServiceAccount("test", testNS, gServiceAccount, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudAuditLogsSource", - Name: sourceName, - UID: sourceUID, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - }}, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, sourceName, true), - }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", sourceName), - Eventf(corev1.EventTypeWarning, "UpdateFailed", `Failed to update status for "%s": invalid value: test@test, serviceAccount should have format: ^[a-z][a-z0-9-]{5,29}@[a-z][a-z0-9-]{5,29}.iam.gserviceaccount.com$: spec.serviceAccount`, sourceName), - }, - WantErr: true, }, { Name: "topic created, not yet been reconciled", Objects: []runtime.Object{ @@ -1089,15 +1047,25 @@ func TestAllCases(t *testing.T) { NewCloudAuditLogsSource(sourceName, testNS, WithCloudAuditLogsSourceSink(sinkGVK, sinkName), WithCloudAuditLogsSourceMethodName(testMethodName), + WithCloudAuditLogsSourceServiceName(testServiceName), WithInitCloudAuditLogsSourceConditions, WithCloudAuditLogsSourceGCPServiceAccount(gServiceAccount), WithCloudAuditLogsSourceDeletionTimestamp, ), }, - Key: testNS + "/" + sourceName, - WantStatusUpdates: nil, + Key: testNS + "/" + sourceName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewCloudAuditLogsSource(sourceName, testNS, + WithCloudAuditLogsSourceMethodName(testMethodName), + WithCloudAuditLogsSourceServiceName(testServiceName), + WithCloudAuditLogsSourceSink(sinkGVK, sinkName), + WithInitCloudAuditLogsSourceConditions, + WithCloudAuditLogsSourceGCPServiceAccount(gServiceAccount), + WithCloudAuditLogsSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123" not found`), + WithCloudAuditLogsSourceDeletionTimestamp), + }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudAuditLogsSource workload identity: getting k8s service account failed with: serviceaccounts "test" not found`), + Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudAuditLogsSource workload identity: getting k8s service account failed with: serviceaccounts "test123" not found`), }, }} diff --git a/pkg/reconciler/events/pubsub/pubsub_test.go b/pkg/reconciler/events/pubsub/pubsub_test.go index 8befe89212..01ed7e8861 100644 --- a/pkg/reconciler/events/pubsub/pubsub_test.go +++ b/pkg/reconciler/events/pubsub/pubsub_test.go @@ -74,7 +74,7 @@ var ( Key: "key.json", } - gServiceAccount = "test@test" + gServiceAccount = "test123@test123.iam.gserviceaccount.com" ) func init() { @@ -125,6 +125,7 @@ func newSink() *unstructured.Unstructured { } } +// TODO add a unit test for successfully creating a k8s service account, after issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { attempts := 0 pubsubSinkURL := sinkURI @@ -137,52 +138,6 @@ func TestAllCases(t *testing.T) { Name: "key not found", // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", - }, { - Name: "k8s serviceaccount created", - Objects: []runtime.Object{ - NewCloudPubSubSource(pubsubName, testNS, - WithCloudPubSubSourceObjectMetaGeneration(generation), - WithCloudPubSubSourceTopic(testTopicID), - WithCloudPubSubSourceSink(sinkGVK, sinkName), - WithCloudPubSubSourceGCPServiceAccount(gServiceAccount), - ), - newSink(), - }, - Key: testNS + "/" + pubsubName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudPubSubSource(pubsubName, testNS, - WithCloudPubSubSourceObjectMetaGeneration(generation), - WithCloudPubSubSourceStatusObservedGeneration(generation), - WithCloudPubSubSourceTopic(testTopicID), - WithCloudPubSubSourceSink(sinkGVK, sinkName), - WithInitCloudPubSubSourceConditions, - WithCloudPubSubSourceObjectMetaGeneration(generation), - WithCloudPubSubSourceGCPServiceAccount(gServiceAccount), - ), - }}, - WantCreates: []runtime.Object{ - NewServiceAccount("test", testNS, gServiceAccount), - }, - WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewServiceAccount("test", testNS, gServiceAccount, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudPubSubSource", - Name: pubsubName, - UID: pubsubUID, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - }}, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, pubsubName, true), - }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", pubsubName), - Eventf(corev1.EventTypeWarning, "UpdateFailed", `Failed to update status for "%s": invalid value: test@test, serviceAccount should have format: ^[a-z][a-z0-9-]{5,29}@[a-z][a-z0-9-]{5,29}.iam.gserviceaccount.com$: spec.serviceAccount`, pubsubName), - }, - WantErr: true, }, { Name: "pullsubscription created", Objects: []runtime.Object{ @@ -357,10 +312,19 @@ func TestAllCases(t *testing.T) { ), newSink(), }, - Key: testNS + "/" + pubsubName, - WantStatusUpdates: nil, + Key: testNS + "/" + pubsubName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewCloudPubSubSource(pubsubName, testNS, + WithCloudPubSubSourceObjectMetaGeneration(generation), + WithCloudPubSubSourceTopic(testTopicID), + WithCloudPubSubSourceSink(sinkGVK, sinkName), + WithCloudPubSubSourceDeletionTimestamp, + WithCloudPubSubSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123" not found`), + WithCloudPubSubSourceGCPServiceAccount(gServiceAccount), + ), + }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudPubSubSource workload identity: getting k8s service account failed with: serviceaccounts "test" not found`), + Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudPubSubSource workload identity: getting k8s service account failed with: serviceaccounts "test123" not found`), }, }} diff --git a/pkg/reconciler/events/scheduler/scheduler_test.go b/pkg/reconciler/events/scheduler/scheduler_test.go index 648e50150b..ffe3bc205d 100644 --- a/pkg/reconciler/events/scheduler/scheduler_test.go +++ b/pkg/reconciler/events/scheduler/scheduler_test.go @@ -90,7 +90,7 @@ var ( Key: "key.json", } - gServiceAccount = "test@test" + gServiceAccount = "test123@test123.iam.gserviceaccount.com" ) func init() { @@ -141,6 +141,7 @@ func newSink() *unstructured.Unstructured { } } +// TODO add a unit test for successfully creating a k8s service account, after issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { schedulerSinkURL := sinkURI @@ -152,52 +153,6 @@ func TestAllCases(t *testing.T) { Name: "key not found", // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", - }, { - Name: "k8s service account created", - Objects: []runtime.Object{ - NewCloudSchedulerSource(schedulerName, testNS, - WithCloudSchedulerSourceSink(sinkGVK, sinkName), - WithCloudSchedulerSourceLocation(location), - WithCloudSchedulerSourceData(testData), - WithCloudSchedulerSourceSchedule(onceAMinuteSchedule), - WithCloudSchedulerSourceGCPServiceAccount(gServiceAccount), - ), - newSink(), - }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudSchedulerSource(schedulerName, testNS, - WithCloudSchedulerSourceSink(sinkGVK, sinkName), - WithCloudSchedulerSourceLocation(location), - WithCloudSchedulerSourceData(testData), - WithCloudSchedulerSourceSchedule(onceAMinuteSchedule), - WithInitCloudSchedulerSourceConditions, - WithCloudSchedulerSourceGCPServiceAccount(gServiceAccount), - ), - }}, - WantCreates: []runtime.Object{ - NewServiceAccount("test", testNS, gServiceAccount), - }, - WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewServiceAccount("test", testNS, gServiceAccount, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudSchedulerSource", - Name: "my-test-scheduler", - UID: schedulerUID, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - }}, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, schedulerName, true), - }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", schedulerName), - Eventf(corev1.EventTypeWarning, "UpdateFailed", `Failed to update status for "%s": invalid value: test@test, serviceAccount should have format: ^[a-z][a-z0-9-]{5,29}@[a-z][a-z0-9-]{5,29}.iam.gserviceaccount.com$: spec.serviceAccount`, schedulerName), - }, - WantErr: true, }, { Name: "topic created, not ready", Objects: []runtime.Object{ @@ -1014,10 +969,23 @@ func TestAllCases(t *testing.T) { ), newSink(), }, - Key: testNS + "/" + schedulerName, - WantStatusUpdates: nil, + Key: testNS + "/" + schedulerName, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewCloudSchedulerSource(schedulerName, testNS, + WithCloudSchedulerSourceProject(testProject), + WithCloudSchedulerSourceSink(sinkGVK, sinkName), + WithCloudSchedulerSourceLocation(location), + WithCloudSchedulerSourceData(testData), + WithCloudSchedulerSourceSchedule(onceAMinuteSchedule), + WithInitCloudSchedulerSourceConditions, + WithCloudSchedulerSourceSinkURI(schedulerSinkURL), + WithCloudSchedulerSourceDeletionTimestamp, + WithCloudSchedulerSourceGCPServiceAccount(gServiceAccount), + WithCloudSchedulerSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123" not found`), + ), + }}, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudSchedulerSource workload identity: getting k8s service account failed with: serviceaccounts "test" not found`), + Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudSchedulerSource workload identity: getting k8s service account failed with: serviceaccounts "test123" not found`), }, }} diff --git a/pkg/reconciler/events/storage/storage_test.go b/pkg/reconciler/events/storage/storage_test.go index 2188401f66..2c49a6ed80 100644 --- a/pkg/reconciler/events/storage/storage_test.go +++ b/pkg/reconciler/events/storage/storage_test.go @@ -89,7 +89,7 @@ var ( Key: "key.json", } - gServiceAccount = "test@test" + gServiceAccount = "test123@test123.iam.gserviceaccount.com" ) func init() { @@ -140,6 +140,7 @@ func newSink() *unstructured.Unstructured { } } +// TODO add a unit test for successfully creating a k8s service account, after issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { storageSinkURL := sinkURI @@ -151,51 +152,6 @@ func TestAllCases(t *testing.T) { Name: "key not found", // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", - }, { - Name: "k8s service account created", - Objects: []runtime.Object{ - NewCloudStorageSource(storageName, testNS, - WithCloudStorageSourceObjectMetaGeneration(generation), - WithCloudStorageSourceBucket(bucket), - WithCloudStorageSourceSink(sinkGVK, sinkName), - WithCloudStorageSourceGCPServiceAccount(gServiceAccount), - ), - newSink(), - }, - Key: testNS + "/" + storageName, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewCloudStorageSource(storageName, testNS, - WithCloudStorageSourceObjectMetaGeneration(generation), - WithCloudStorageSourceStatusObservedGeneration(generation), - WithCloudStorageSourceBucket(bucket), - WithCloudStorageSourceSink(sinkGVK, sinkName), - WithInitCloudStorageSourceConditions, - WithCloudStorageSourceGCPServiceAccount(gServiceAccount), - ), - }}, - WantCreates: []runtime.Object{ - NewServiceAccount("test", testNS, gServiceAccount), - }, - WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewServiceAccount("test", testNS, gServiceAccount, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "events.cloud.google.com/v1alpha1", - Kind: "CloudStorageSource", - Name: "my-test-storage", - UID: storageUID, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - }}, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, storageName, true), - }, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", storageName), - Eventf(corev1.EventTypeWarning, "UpdateFailed", `Failed to update status for "%s": invalid value: test@test, serviceAccount should have format: ^[a-z][a-z0-9-]{5,29}@[a-z][a-z0-9-]{5,29}.iam.gserviceaccount.com$: spec.serviceAccount`, storageName), - }, - WantErr: true, }, { Name: "topic created, not yet been reconciled", Objects: []runtime.Object{ @@ -892,9 +848,20 @@ func TestAllCases(t *testing.T) { }, Key: testNS + "/" + storageName, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudStorageSource workload identity: getting k8s service account failed with: serviceaccounts "test" not found`), + Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete CloudStorageSource workload identity: getting k8s service account failed with: serviceaccounts "test123" not found`), }, - WantStatusUpdates: nil, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewCloudStorageSource(storageName, testNS, + WithCloudStorageSourceProject(testProject), + WithCloudStorageSourceObjectMetaGeneration(generation), + WithCloudStorageSourceBucket(bucket), + WithCloudStorageSourceSink(sinkGVK, sinkName), + WithCloudStorageSourceSinkURI(storageSinkURL), + WithCloudStorageSourceGCPServiceAccount(gServiceAccount), + WithCloudStorageSourceWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123" not found`), + WithDeletionTimestamp(), + ), + }}, }, { Name: "successfully deleted storage", Objects: []runtime.Object{ diff --git a/pkg/reconciler/identity/reconciler.go b/pkg/reconciler/identity/reconciler.go index e2c9af1900..2e8f13408e 100644 --- a/pkg/reconciler/identity/reconciler.go +++ b/pkg/reconciler/identity/reconciler.go @@ -38,9 +38,11 @@ import ( ) const ( - Add = "add" - Remove = "remove" - Role = "roles/iam.workloadIdentityUser" + Add = "add" + Remove = "remove" + Role = "roles/iam.workloadIdentityUser" + deleteWorkloadIdentityFailed = "WorkloadIdentityDeleteFailed" + workloadIdentityFailed = "WorkloadIdentityReconcileFailed" ) func NewIdentity(ctx context.Context) *Identity { @@ -56,12 +58,15 @@ type Identity struct { // ReconcileWorkloadIdentity will create a k8s service account, add ownerReference to it, // and add iam policy binding between this k8s service account and its corresponding GCP service account. func (i *Identity) ReconcileWorkloadIdentity(ctx context.Context, projectID string, identifiable duck.Identifiable) (*corev1.ServiceAccount, error) { + status := identifiable.IdentityStatus() // Create corresponding k8s ServiceAccount if it doesn't exist. namespace := identifiable.GetObjectMeta().GetNamespace() - kServiceAccount, err := i.createServiceAccount(ctx, namespace, identifiable.GetIdentity()) + kServiceAccount, err := i.createServiceAccount(ctx, namespace, identifiable.IdentitySpec().ServiceAccount) if err != nil { + status.MarkWorkloadIdentityFailed(identifiable.ConditionSet(), workloadIdentityFailed, err.Error()) return nil, fmt.Errorf("failed to get k8s ServiceAccount: %w", err) } + status.ServiceAccountName = kServiceAccount.Name // Add ownerReference to K8s ServiceAccount. expectOwnerReference := *kmeta.NewControllerRef(identifiable) expectOwnerReference.Controller = ptr.Bool(false) @@ -69,30 +74,36 @@ func (i *Identity) ReconcileWorkloadIdentity(ctx context.Context, projectID stri kServiceAccount.OwnerReferences = append(kServiceAccount.OwnerReferences, expectOwnerReference) if _, err := i.KubeClient.CoreV1().ServiceAccounts(kServiceAccount.Namespace).Update(kServiceAccount); err != nil { logging.FromContext(ctx).Desugar().Error("Failed to update OwnerReferences", zap.Error(err)) + status.MarkWorkloadIdentityFailed(identifiable.ConditionSet(), workloadIdentityFailed, err.Error()) return nil, fmt.Errorf("failed to update OwnerReferences: %w", err) } } // Add iam policy binding to GCP ServiceAccount. - if err := addIamPolicyBinding(ctx, projectID, identifiable.GetIdentity(), kServiceAccount); err != nil { + if err := addIamPolicyBinding(ctx, projectID, identifiable.IdentitySpec().ServiceAccount, kServiceAccount); err != nil { + status.MarkWorkloadIdentityFailed(identifiable.ConditionSet(), workloadIdentityFailed, err.Error()) return kServiceAccount, fmt.Errorf("adding iam policy binding failed with: %s", err) } + status.MarkWorkloadIdentityConfigured(identifiable.ConditionSet()) return kServiceAccount, nil } // DeleteWorkloadIdentity will remove iam policy binding between k8s service account and its corresponding GCP service account, // if this k8s service account only has one ownerReference. func (i *Identity) DeleteWorkloadIdentity(ctx context.Context, projectID string, identifiable duck.Identifiable) error { + status := identifiable.IdentityStatus() namespace := identifiable.GetObjectMeta().GetNamespace() - kServiceAccountName := resources.GenerateServiceAccountName(identifiable.GetIdentity()) + kServiceAccountName := resources.GenerateServiceAccountName(identifiable.IdentitySpec().ServiceAccount) kServiceAccount, err := i.KubeClient.CoreV1().ServiceAccounts(namespace).Get(kServiceAccountName, metav1.GetOptions{}) if err != nil { + status.MarkWorkloadIdentityFailed(identifiable.ConditionSet(), deleteWorkloadIdentityFailed, err.Error()) // k8s ServiceAccount should be there. return fmt.Errorf("getting k8s service account failed with: %w", err) } if kServiceAccount != nil && len(kServiceAccount.OwnerReferences) == 1 { logging.FromContext(ctx).Desugar().Debug("Removing iam policy binding.") - if err := removeIamPolicyBinding(ctx, projectID, identifiable.GetIdentity(), kServiceAccount); err != nil { + if err := removeIamPolicyBinding(ctx, projectID, identifiable.IdentitySpec().ServiceAccount, kServiceAccount); err != nil { + status.MarkWorkloadIdentityFailed(identifiable.ConditionSet(), deleteWorkloadIdentityFailed, err.Error()) return fmt.Errorf("removing iam policy binding failed with: %w", err) } } diff --git a/pkg/reconciler/messaging/channel/channel_test.go b/pkg/reconciler/messaging/channel/channel_test.go index 59f71f752f..7db4da1409 100644 --- a/pkg/reconciler/messaging/channel/channel_test.go +++ b/pkg/reconciler/messaging/channel/channel_test.go @@ -80,7 +80,7 @@ var ( replyDNS = "reply.mynamespace.svc.cluster.local" replyURI = apis.HTTP(replyDNS) - gServiceAccount = "test@test" + gServiceAccount = "test123@test123.iam.gserviceaccount.com" ) func init() { @@ -101,6 +101,7 @@ func patchFinalizers(namespace, name string, add bool) clientgotesting.PatchActi return action } +// TODO add a unit test for successfully creating a k8s service account, after issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { table := TableTest{{ Name: "bad workqueue key", @@ -110,51 +111,6 @@ func TestAllCases(t *testing.T) { Name: "key not found", // Make sure Reconcile handles good keys that don't exist. Key: "foo/not-found", - }, { - Name: "k8s service account created", - Objects: []runtime.Object{ - NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithChannelGCPServiceAccount(gServiceAccount), - ), - }, - Key: testNS + "/" + channelName, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "FinalizerUpdate", "Updated %q finalizers", channelName), - Eventf(corev1.EventTypeWarning, "UpdateFailed", `Failed to update status for "%s": invalid value: test@test, serviceAccount should have format: ^[a-z][a-z0-9-]{5,29}@[a-z][a-z0-9-]{5,29}.iam.gserviceaccount.com$: spec.serviceAccount`, channelName), - }, - WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewChannel(channelName, testNS, - WithChannelUID(channelUID), - WithInitChannelConditions, - WithChannelSpec(v1alpha1.ChannelSpec{ - Project: testProject, - }), - WithChannelGCPServiceAccount(gServiceAccount), - ), - }}, - WantCreates: []runtime.Object{ - NewServiceAccount("test", testNS, gServiceAccount), - }, - WantUpdates: []clientgotesting.UpdateActionImpl{{ - Object: NewServiceAccount("test", testNS, gServiceAccount, - WithServiceAccountOwnerReferences([]metav1.OwnerReference{{ - APIVersion: "messaging.cloud.google.com/v1alpha1", - Kind: "Channel", - Name: "chan", - UID: channelUID, - Controller: &falseVal, - BlockOwnerDeletion: &trueVal, - }}), - ), - }}, - WantPatches: []clientgotesting.PatchActionImpl{ - patchFinalizers(testNS, channelName, true), - }, - WantErr: true, }, { Name: "create topic", Objects: []runtime.Object{ @@ -546,16 +502,28 @@ func TestAllCases(t *testing.T) { Project: testProject, }), WithInitChannelConditions, - WithChannelDefaults, WithChannelGCPServiceAccount(gServiceAccount), + WithChannelDefaults, WithChannelDeletionTimestamp, ), }, Key: testNS + "/" + channelName, WantEvents: []string{ - Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete Channel workload identity: getting k8s service account failed with: serviceaccounts "test" not found`), + Eventf(corev1.EventTypeWarning, "WorkloadIdentityDeleteFailed", `Failed to delete Channel workload identity: getting k8s service account failed with: serviceaccounts "test123" not found`), }, - WantStatusUpdates: nil, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewChannel(channelName, testNS, + WithChannelUID(channelUID), + WithChannelSpec(v1alpha1.ChannelSpec{ + Project: testProject, + }), + WithInitChannelConditions, + WithChannelGCPServiceAccount(gServiceAccount), + WithChannelDefaults, + WithChannelDeletionTimestamp, + WithChannelWorkloadIdentityFailed("WorkloadIdentityDeleteFailed", `serviceaccounts "test123" not found`), + ), + }}, }} defer logtesting.ClearAll() diff --git a/pkg/reconciler/testing/auditlogs.go b/pkg/reconciler/testing/auditlogs.go index 1129831efa..f8cf048d2c 100644 --- a/pkg/reconciler/testing/auditlogs.go +++ b/pkg/reconciler/testing/auditlogs.go @@ -49,38 +49,44 @@ func WithInitCloudAuditLogsSourceConditions(s *v1alpha1.CloudAuditLogsSource) { func WithCloudAuditLogsSourceTopicFailed(reason, message string) CloudAuditLogsSourceOption { return func(s *v1alpha1.CloudAuditLogsSource) { - s.Status.MarkTopicFailed(reason, message) + s.Status.MarkTopicFailed(s.ConditionSet(), reason, message) + } +} + +func WithCloudAuditLogsSourceWorkloadIdentityFailed(reason, message string) CloudAuditLogsSourceOption { + return func(s *v1alpha1.CloudAuditLogsSource) { + s.Status.MarkWorkloadIdentityFailed(s.ConditionSet(), reason, message) } } func WithCloudAuditLogsSourceTopicUnknown(reason, message string) CloudAuditLogsSourceOption { return func(s *v1alpha1.CloudAuditLogsSource) { - s.Status.MarkTopicUnknown(reason, message) + s.Status.MarkTopicUnknown(s.ConditionSet(), reason, message) } } func WithCloudAuditLogsSourceTopicReady(topicID string) CloudAuditLogsSourceOption { return func(s *v1alpha1.CloudAuditLogsSource) { - s.Status.MarkTopicReady() + s.Status.MarkTopicReady(s.ConditionSet()) s.Status.TopicID = topicID } } func WithCloudAuditLogsSourcePullSubscriptionFailed(reason, message string) CloudAuditLogsSourceOption { return func(s *v1alpha1.CloudAuditLogsSource) { - s.Status.MarkPullSubscriptionFailed(reason, message) + s.Status.MarkPullSubscriptionFailed(s.ConditionSet(), reason, message) } } func WithCloudAuditLogsSourcePullSubscriptionUnknown(reason, message string) CloudAuditLogsSourceOption { return func(s *v1alpha1.CloudAuditLogsSource) { - s.Status.MarkPullSubscriptionUnknown(reason, message) + s.Status.MarkPullSubscriptionUnknown(s.ConditionSet(), reason, message) } } func WithCloudAuditLogsSourcePullSubscriptionReady() CloudAuditLogsSourceOption { return func(s *v1alpha1.CloudAuditLogsSource) { - s.Status.MarkPullSubscriptionReady() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) } } diff --git a/pkg/reconciler/testing/channel.go b/pkg/reconciler/testing/channel.go index e3fc9635d7..6ac82292c5 100644 --- a/pkg/reconciler/testing/channel.go +++ b/pkg/reconciler/testing/channel.go @@ -79,6 +79,12 @@ func WithInitChannelConditions(s *v1alpha1.Channel) { s.Status.InitializeConditions() } +func WithChannelWorkloadIdentityFailed(reason, message string) ChannelOption { + return func(s *v1alpha1.Channel) { + s.Status.MarkWorkloadIdentityFailed(s.ConditionSet(), reason, message) + } +} + func WithChannelTopic(topicID string) ChannelOption { return func(s *v1alpha1.Channel) { s.Status.MarkTopicReady() diff --git a/pkg/reconciler/testing/pubsub.go b/pkg/reconciler/testing/pubsub.go index b2a4294200..0c0424661c 100644 --- a/pkg/reconciler/testing/pubsub.go +++ b/pkg/reconciler/testing/pubsub.go @@ -87,6 +87,12 @@ func WithInitCloudPubSubSourceConditions(ps *v1alpha1.CloudPubSubSource) { ps.Status.InitializeConditions() } +func WithCloudPubSubSourceWorkloadIdentityFailed(reason, message string) CloudPubSubSourceOption { + return func(s *v1alpha1.CloudPubSubSource) { + s.Status.MarkWorkloadIdentityFailed(s.ConditionSet(), reason, message) + } +} + // WithCloudPubSubSourcePullSubscriptionFailed marks the condition that the // status of PullSubscription is False func WithCloudPubSubSourcePullSubscriptionFailed(reason, message string) CloudPubSubSourceOption { diff --git a/pkg/reconciler/testing/scheduler.go b/pkg/reconciler/testing/scheduler.go index 8450421575..13c7f5ec9a 100644 --- a/pkg/reconciler/testing/scheduler.go +++ b/pkg/reconciler/testing/scheduler.go @@ -99,11 +99,17 @@ func WithInitCloudSchedulerSourceConditions(s *v1alpha1.CloudSchedulerSource) { s.Status.InitializeConditions() } +func WithCloudSchedulerSourceWorkloadIdentityFailed(reason, message string) CloudSchedulerSourceOption { + return func(s *v1alpha1.CloudSchedulerSource) { + s.Status.MarkWorkloadIdentityFailed(s.ConditionSet(), reason, message) + } +} + // WithCloudSchedulerSourceTopicFailed marks the condition that the // status of topic is False. func WithCloudSchedulerSourceTopicFailed(reason, message string) CloudSchedulerSourceOption { return func(s *v1alpha1.CloudSchedulerSource) { - s.Status.MarkTopicFailed(reason, message) + s.Status.MarkTopicFailed(s.ConditionSet(), reason, message) } } @@ -111,7 +117,7 @@ func WithCloudSchedulerSourceTopicFailed(reason, message string) CloudSchedulerS // status of topic is Unknown. func WithCloudSchedulerSourceTopicUnknown(reason, message string) CloudSchedulerSourceOption { return func(s *v1alpha1.CloudSchedulerSource) { - s.Status.MarkTopicUnknown(reason, message) + s.Status.MarkTopicUnknown(s.ConditionSet(), reason, message) } } @@ -119,7 +125,9 @@ func WithCloudSchedulerSourceTopicUnknown(reason, message string) CloudScheduler // topic is not ready. func WithCloudSchedulerSourceTopicReady(topicID, projectID string) CloudSchedulerSourceOption { return func(s *v1alpha1.CloudSchedulerSource) { - s.Status.MarkTopicReady(topicID, projectID) + s.Status.MarkTopicReady(s.ConditionSet()) + s.Status.TopicID = topicID + s.Status.ProjectID = projectID } } @@ -127,7 +135,7 @@ func WithCloudSchedulerSourceTopicReady(topicID, projectID string) CloudSchedule // topic is False. func WithCloudSchedulerSourcePullSubscriptionFailed(reason, message string) CloudSchedulerSourceOption { return func(s *v1alpha1.CloudSchedulerSource) { - s.Status.MarkPullSubscriptionFailed(reason, message) + s.Status.MarkPullSubscriptionFailed(s.ConditionSet(), reason, message) } } @@ -135,7 +143,7 @@ func WithCloudSchedulerSourcePullSubscriptionFailed(reason, message string) Clou // topic is Unknown. func WithCloudSchedulerSourcePullSubscriptionUnknown(reason, message string) CloudSchedulerSourceOption { return func(s *v1alpha1.CloudSchedulerSource) { - s.Status.MarkPullSubscriptionUnknown(reason, message) + s.Status.MarkPullSubscriptionUnknown(s.ConditionSet(), reason, message) } } @@ -143,7 +151,7 @@ func WithCloudSchedulerSourcePullSubscriptionUnknown(reason, message string) Clo // topic is ready. func WithCloudSchedulerSourcePullSubscriptionReady() CloudSchedulerSourceOption { return func(s *v1alpha1.CloudSchedulerSource) { - s.Status.MarkPullSubscriptionReady() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) } } diff --git a/pkg/reconciler/testing/storage.go b/pkg/reconciler/testing/storage.go index 8ddafaf9ab..117f23b59c 100644 --- a/pkg/reconciler/testing/storage.go +++ b/pkg/reconciler/testing/storage.go @@ -82,6 +82,12 @@ func WithInitCloudStorageSourceConditions(s *v1alpha1.CloudStorageSource) { s.Status.InitializeConditions() } +func WithCloudStorageSourceWorkloadIdentityFailed(reason, message string) CloudStorageSourceOption { + return func(s *v1alpha1.CloudStorageSource) { + s.Status.MarkWorkloadIdentityFailed(s.ConditionSet(), reason, message) + } +} + func WithCloudStorageSourceGCPServiceAccount(gServiceAccount string) CloudStorageSourceOption { return func(ps *v1alpha1.CloudStorageSource) { ps.Spec.ServiceAccount = gServiceAccount @@ -92,7 +98,7 @@ func WithCloudStorageSourceGCPServiceAccount(gServiceAccount string) CloudStorag // topic is False func WithCloudStorageSourceTopicFailed(reason, message string) CloudStorageSourceOption { return func(s *v1alpha1.CloudStorageSource) { - s.Status.MarkTopicFailed(reason, message) + s.Status.MarkTopicFailed(s.ConditionSet(), reason, message) } } @@ -100,7 +106,7 @@ func WithCloudStorageSourceTopicFailed(reason, message string) CloudStorageSourc // topic is False func WithCloudStorageSourceTopicUnknown(reason, message string) CloudStorageSourceOption { return func(s *v1alpha1.CloudStorageSource) { - s.Status.MarkTopicUnknown(reason, message) + s.Status.MarkTopicUnknown(s.ConditionSet(), reason, message) } } @@ -108,7 +114,7 @@ func WithCloudStorageSourceTopicUnknown(reason, message string) CloudStorageSour // topic is not ready func WithCloudStorageSourceTopicReady(topicID string) CloudStorageSourceOption { return func(s *v1alpha1.CloudStorageSource) { - s.Status.MarkTopicReady() + s.Status.MarkTopicReady(s.ConditionSet()) s.Status.TopicID = topicID } } @@ -123,7 +129,7 @@ func WithCloudStorageSourceTopicID(topicID string) CloudStorageSourceOption { // status of topic is False func WithCloudStorageSourcePullSubscriptionFailed(reason, message string) CloudStorageSourceOption { return func(s *v1alpha1.CloudStorageSource) { - s.Status.MarkPullSubscriptionFailed(reason, message) + s.Status.MarkPullSubscriptionFailed(s.ConditionSet(), reason, message) } } @@ -131,7 +137,7 @@ func WithCloudStorageSourcePullSubscriptionFailed(reason, message string) CloudS // status of topic is Unknown. func WithCloudStorageSourcePullSubscriptionUnknown(reason, message string) CloudStorageSourceOption { return func(s *v1alpha1.CloudStorageSource) { - s.Status.MarkPullSubscriptionUnknown(reason, message) + s.Status.MarkPullSubscriptionUnknown(s.ConditionSet(), reason, message) } } @@ -139,7 +145,7 @@ func WithCloudStorageSourcePullSubscriptionUnknown(reason, message string) Cloud // topic is ready. func WithCloudStorageSourcePullSubscriptionReady() CloudStorageSourceOption { return func(s *v1alpha1.CloudStorageSource) { - s.Status.MarkPullSubscriptionReady() + s.Status.MarkPullSubscriptionReady(s.ConditionSet()) } }