From 9118dd305d9af34b47cfe22320be553db6aa2710 Mon Sep 17 00:00:00 2001 From: grac3gao Date: Wed, 25 Mar 2020 21:48:38 -0700 Subject: [PATCH 1/8] status --- pkg/apis/duck/v1alpha1/identity_lifecycle.go | 32 ++++ .../duck/v1alpha1/identity_lifecycle_test.go | 55 +++++++ pkg/apis/duck/v1alpha1/identity_types.go | 103 ++++++++++++ pkg/apis/duck/v1alpha1/identity_types_test.go | 67 ++++++++ pkg/apis/duck/v1alpha1/pubsub_types.go | 12 +- .../duck/v1alpha1/zz_generated.deepcopy.go | 103 +++++++++++- .../cloudauditlogssource_lifecycle.go | 33 ---- .../cloudauditlogssource_lifecycle_test.go | 72 ++++----- .../v1alpha1/cloudauditlogssource_types.go | 25 +-- .../cloudauditlogssource_types_test.go | 21 ++- .../cloudauditlogssource_validation_test.go | 4 +- .../v1alpha1/cloudpubsubsource_types.go | 35 ++-- .../v1alpha1/cloudpubsubsource_types_test.go | 21 ++- .../cloudpubsubsource_validation_test.go | 4 +- .../cloudschedulersource_lifecycle.go | 38 ----- .../cloudschedulersource_lifecycle_test.go | 78 ++++----- .../v1alpha1/cloudschedulersource_types.go | 25 +-- .../cloudschedulersource_types_test.go | 21 ++- .../cloudschedulersource_validation_test.go | 16 +- .../v1alpha1/cloudstoragesource_lifecycle.go | 34 ---- .../cloudstoragesource_lifecycle_test.go | 72 ++++----- .../v1alpha1/cloudstoragesource_types.go | 25 +-- .../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 | 27 ++-- .../messaging/v1alpha1/channel_types_test.go | 22 ++- .../v1alpha1/channel_validation_test.go | 20 ++- .../v1alpha1/zz_generated.deepcopy.go | 3 +- .../ducks/duck/v1alpha1/identity/fake/fake.go | 30 ++++ .../ducks/duck/v1alpha1/identity/identity.go | 60 +++++++ pkg/duck/identifiable.go | 11 +- 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 | 24 ++- .../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 ++- 45 files changed, 1027 insertions(+), 660 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 create mode 100644 pkg/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go create mode 100644 pkg/client/injection/ducks/duck/v1alpha1/identity/identity.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..92a4fcd810 --- /dev/null +++ b/pkg/apis/duck/v1alpha1/identity_lifecycle.go @@ -0,0 +1,32 @@ +/* +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. + 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..43d8c1aa1a --- /dev/null +++ b/pkg/apis/duck/v1alpha1/identity_types.go @@ -0,0 +1,103 @@ +/* +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 ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + "knative.dev/pkg/apis" + "knative.dev/pkg/apis/duck" + duckv1 "knative.dev/pkg/apis/duck/v1" +) + +// PubSub is an Implementable "duck type". +var _ duck.Implementable = (*Identity)(nil) + +// +genduck +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +type Identity struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec PubSubSpec `json:"spec"` + Status PubSubStatus `json:"status"` +} + +type IdentitySpec struct { + // gServiceAccount 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"` +} + +// PubSubStatus shows how we expect folks to embed Addressable in +// their Status field. +type IdentityStatus struct { + // inherits IdentityStatus, 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 +} + +var ( + // Verify PubSub resources meet duck contracts. + _ duck.Populatable = (*Identity)(nil) + _ apis.Listable = (*Identity)(nil) +) + +// GetFullType implements duck.Implementable +func (*Identity) GetFullType() duck.Populatable { + return &Identity{} +} + +// Populate implements duck.Populatable +func (s *Identity) Populate() { + s.Spec.ServiceAccount = "" +} + +// GetListType implements apis.Listable +func (*Identity) GetListType() runtime.Object { + return &IdentityList{} +} + +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// PubSubList is a list of PubSub resources +type IdentityList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata"` + + Items []Identity `json:"items"` +} 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..f5cd10a4dd --- /dev/null +++ b/pkg/apis/duck/v1alpha1/identity_types_test.go @@ -0,0 +1,67 @@ +/* +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 TestIdentityGetListType(t *testing.T) { + c := &Identity{} + switch c.GetListType().(type) { + case *IdentityList: + // expected + default: + t.Errorf("expected GetListType to return *ChannelableList, got %T", c.GetListType()) + } +} + +func TestIdentityPopulate(t *testing.T) { + got := &Identity{} + want := &Identity{ + Spec: PubSubSpec{}, + } + + got.Populate() + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Unexpected difference (-want, +got): %v", diff) + } +} + +func TestGetFullType(t *testing.T) { + got := &Identity{} + want := &Identity{} + + got.GetFullType() + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Unexpected difference (-want, +got): %v", diff) + } +} + +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 2dea3381a0..494c1417d1 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,8 +68,11 @@ type PubSubSpec struct { // PubSubStatus shows how we expect folks to embed Addressable in // their Status field. type PubSubStatus struct { - // This brings in duck/v1beta1 Status as well as SinkURI - duckv1.SourceStatus + IdentityStatus `json:",inline"` + + // SinkURI is the current active sink URI that has been configured for the Source. + // +optional + SinkURI *apis.URL `json:"sinkUri,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..efd654b87b 100644 --- a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go @@ -23,8 +23,103 @@ package v1alpha1 import ( v1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" + apis "knative.dev/pkg/apis" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Identity) DeepCopyInto(out *Identity) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Identity. +func (in *Identity) DeepCopy() *Identity { + if in == nil { + return nil + } + out := new(Identity) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *Identity) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IdentityList) DeepCopyInto(out *IdentityList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]Identity, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IdentityList. +func (in *IdentityList) DeepCopy() *IdentityList { + if in == nil { + return nil + } + out := new(IdentityList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *IdentityList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// 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 +185,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 +207,12 @@ 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) + } 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..640fdc9227 100644 --- a/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go +++ b/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go @@ -107,16 +107,14 @@ func (*CloudAuditLogsSource) GetGroupVersionKind() schema.GroupVersionKind { return SchemeGroupVersion.WithKind("CloudAuditLogsSource") } -///Methods for pubsubable interface +// Methods for identifiable interface. -// PubSubSpec returns the PubSubSpec portion of the Spec. -func (s *CloudAuditLogsSource) PubSubSpec() *duckv1alpha1.PubSubSpec { - return &s.Spec.PubSubSpec +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 +func (s *CloudAuditLogsSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -124,9 +122,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..fae8dbaf9b 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,14 @@ func (s *CloudPubSubSource) GetGroupVersionKind() schema.GroupVersionKind { return SchemeGroupVersion.WithKind("CloudPubSubSource") } -// Methods for pubsubable interface +// Methods for identifiable interface. -// CloudPubSubSourceSpec returns the CloudPubSubSourceSpec portion of the Spec. -func (ps *CloudPubSubSource) PubSubSpec() *duckv1alpha1.PubSubSpec { - return &ps.Spec.PubSubSpec +func (s *CloudPubSubSource) IdentitySpec() *duckv1alpha1.IdentitySpec { + return &s.Spec.IdentitySpec +} + +func (s *CloudPubSubSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -158,7 +161,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..c133eb65a2 100644 --- a/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go +++ b/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go @@ -92,16 +92,31 @@ 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) } 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..ae7913217b 100644 --- a/pkg/apis/events/v1alpha1/cloudschedulersource_types.go +++ b/pkg/apis/events/v1alpha1/cloudschedulersource_types.go @@ -110,15 +110,14 @@ 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 + +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 +func (s *CloudSchedulerSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -126,9 +125,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..4011708c67 100644 --- a/pkg/apis/events/v1alpha1/cloudstoragesource_types.go +++ b/pkg/apis/events/v1alpha1/cloudstoragesource_types.go @@ -126,16 +126,14 @@ func (storage *CloudStorageSource) GetGroupVersionKind() schema.GroupVersionKind return SchemeGroupVersion.WithKind("CloudStorageSource") } -// Methods for pubsubable interface +// Methods for identifiable interface. -// PubSubSpec returns the PubSubSpec portion of the Spec. -func (s *CloudStorageSource) PubSubSpec() *duckv1alpha1.PubSubSpec { - return &s.Spec.PubSubSpec +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 +func (s *CloudStorageSource) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &s.Status.IdentityStatus } // ConditionSet returns the apis.ConditionSet of the embedding object @@ -143,9 +141,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..fbacbff4bd 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,18 @@ 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. + +func (c *Channel) IdentitySpec() *duckv1alpha1.IdentitySpec { + return &c.Spec.IdentitySpec +} + +func (c *Channel) IdentityStatus() *duckv1alpha1.IdentityStatus { + return &c.Status.IdentityStatus +} + +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..4d77b22959 100644 --- a/pkg/apis/messaging/v1alpha1/channel_types_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_types_test.go @@ -22,6 +22,7 @@ import ( "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,14 +40,29 @@ 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) } 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/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go b/pkg/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go new file mode 100644 index 0000000000..7d866c948e --- /dev/null +++ b/pkg/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go @@ -0,0 +1,30 @@ +/* +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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package fake + +import ( + identity "github.com/google/knative-gcp/pkg/client/injection/ducks/duck/v1alpha1/identity" + injection "knative.dev/pkg/injection" +) + +var Get = identity.Get + +func init() { + injection.Fake.RegisterDuck(identity.WithDuck) +} diff --git a/pkg/client/injection/ducks/duck/v1alpha1/identity/identity.go b/pkg/client/injection/ducks/duck/v1alpha1/identity/identity.go new file mode 100644 index 0000000000..b3cf4dc84e --- /dev/null +++ b/pkg/client/injection/ducks/duck/v1alpha1/identity/identity.go @@ -0,0 +1,60 @@ +/* +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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package identity + +import ( + context "context" + + v1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" + duck "knative.dev/pkg/apis/duck" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" + dynamicclient "knative.dev/pkg/injection/clients/dynamicclient" + logging "knative.dev/pkg/logging" +) + +func init() { + injection.Default.RegisterDuck(WithDuck) +} + +// Key is used for associating the Informer inside the context.Context. +type Key struct{} + +func WithDuck(ctx context.Context) context.Context { + dc := dynamicclient.Get(ctx) + dif := &duck.CachedInformerFactory{ + Delegate: &duck.TypedInformerFactory{ + Client: dc, + Type: (&v1alpha1.Identity{}).GetFullType(), + ResyncPeriod: controller.GetResyncPeriod(ctx), + StopChannel: ctx.Done(), + }, + } + return context.WithValue(ctx, Key{}, dif) +} + +// Get extracts the typed informer from the context. +func Get(ctx context.Context) duck.InformerFactory { + untyped := ctx.Value(Key{}) + if untyped == nil { + logging.FromContext(ctx).Panic( + "Unable to fetch knative.dev/pkg/apis/duck.InformerFactory from context.") + } + return untyped.(duck.InformerFactory) +} diff --git a/pkg/duck/identifiable.go b/pkg/duck/identifiable.go index 6ee7af556b..8dc4a3c412 100644 --- a/pkg/duck/identifiable.go +++ b/pkg/duck/identifiable.go @@ -16,10 +16,17 @@ 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() *duckv1alpha1.IdentitySpec + IdentityStatus() *duckv1alpha1.IdentityStatus + 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 b4593633a2..e203eae15a 100644 --- a/pkg/reconciler/events/auditlogs/auditlogs_test.go +++ b/pkg/reconciler/events/auditlogs/auditlogs_test.go @@ -96,7 +96,7 @@ var ( Key: "key.json", } - gServiceAccount = "test@test" + gServiceAccount = "test123@test123.iam.gserviceaccount.com" ) func sourceOwnerRef(name string, uid types.UID) metav1.OwnerReference { @@ -132,6 +132,7 @@ func sinkURL(t *testing.T, url string) *apis.URL { return u } +// TODO add a unit test for successfully creating a k8s service account, after issue issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { calSinkURL := sinkURL(t, sinkURI) @@ -142,49 +143,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{ @@ -1097,15 +1055,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 f6777568a2..9ecb4c6eb0 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() { @@ -134,6 +134,7 @@ func sinkURL(t *testing.T, url string) *apis.URL { return u } +// TODO add a unit test for successfully creating a k8s service account, after issue issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { attempts := 0 pubsubSinkURL := sinkURL(t, sinkURI) @@ -146,52 +147,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{ @@ -366,10 +321,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 f95281d265..248201b9cb 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() { @@ -150,6 +150,7 @@ func sinkURL(t *testing.T, url string) *apis.URL { return u } +// TODO add a unit test for successfully creating a k8s service account, after issue issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { schedulerSinkURL := sinkURL(t, sinkURI) @@ -161,52 +162,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{ @@ -1023,10 +978,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 f698d05fa1..83b0a8ec05 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() { @@ -149,6 +149,7 @@ func sinkURL(t *testing.T, url string) *apis.URL { return u } +// TODO add a unit test for successfully creating a k8s service account, after issue issue https://github.com/google/knative-gcp/issues/657 gets solved. func TestAllCases(t *testing.T) { storageSinkURL := sinkURL(t, sinkURI) @@ -160,51 +161,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{ @@ -901,9 +857,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..80967e788e 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,10 +58,12 @@ 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) } // Add ownerReference to K8s ServiceAccount. @@ -69,30 +73,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..5b6ecd2222 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 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()) } } From e51cefa6930b89498fd73e7a64367fc90084029d Mon Sep 17 00:00:00 2001 From: grac3gao Date: Thu, 26 Mar 2020 11:38:10 -0700 Subject: [PATCH 2/8] update code --- pkg/apis/duck/v1alpha1/identity_types.go | 15 +++++++-------- pkg/apis/duck/v1alpha1/identity_types_test.go | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/apis/duck/v1alpha1/identity_types.go b/pkg/apis/duck/v1alpha1/identity_types.go index 43d8c1aa1a..5febc811c7 100644 --- a/pkg/apis/duck/v1alpha1/identity_types.go +++ b/pkg/apis/duck/v1alpha1/identity_types.go @@ -32,21 +32,20 @@ type Identity struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec PubSubSpec `json:"spec"` - Status PubSubStatus `json:"status"` + Spec IdentitySpec `json:"spec"` + Status IdentityStatus `json:"status"` } type IdentitySpec struct { - // gServiceAccount is the GCP service account which has required permissions to poll from a Cloud Pub/Sub subscription. + // 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"` } -// PubSubStatus shows how we expect folks to embed Addressable in -// their Status field. +// IdentityStatus inherits duck/v1 Status and adds a ServiceAccountName. type IdentityStatus struct { - // inherits IdentityStatus, which currently provides: + // 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"` @@ -72,7 +71,7 @@ func (ss *IdentityStatus) IsReady() bool { } var ( - // Verify PubSub resources meet duck contracts. + // Verify Identity resources meet duck contracts. _ duck.Populatable = (*Identity)(nil) _ apis.Listable = (*Identity)(nil) ) @@ -94,7 +93,7 @@ func (*Identity) GetListType() runtime.Object { // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object -// PubSubList is a list of PubSub resources +// IdentityList is a list of PubSub resources type IdentityList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata"` diff --git a/pkg/apis/duck/v1alpha1/identity_types_test.go b/pkg/apis/duck/v1alpha1/identity_types_test.go index f5cd10a4dd..6910d9366b 100644 --- a/pkg/apis/duck/v1alpha1/identity_types_test.go +++ b/pkg/apis/duck/v1alpha1/identity_types_test.go @@ -34,7 +34,7 @@ func TestIdentityGetListType(t *testing.T) { func TestIdentityPopulate(t *testing.T) { got := &Identity{} want := &Identity{ - Spec: PubSubSpec{}, + Spec: IdentitySpec{}, } got.Populate() From d977ed7e77aadd34a95053b6f4d647a851c7e370 Mon Sep 17 00:00:00 2001 From: grac3gao Date: Thu, 26 Mar 2020 11:45:52 -0700 Subject: [PATCH 3/8] hack/update-codegen.sh + UT coverage --- .../duck/v1alpha1/zz_generated.deepcopy.go | 2 +- .../v1alpha1/cloudpubsubsource_types_test.go | 24 +++++++++++++++++ .../messaging/v1alpha1/channel_types_test.go | 26 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go index efd654b87b..de9f59766d 100644 --- a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go @@ -31,7 +31,7 @@ func (in *Identity) DeepCopyInto(out *Identity) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) + out.Spec = in.Spec in.Status.DeepCopyInto(&out.Status) return } diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_types_test.go index c133eb65a2..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" @@ -121,3 +123,25 @@ func TestCloudPubSubSourceIdentityStatus(t *testing.T) { 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/messaging/v1alpha1/channel_types_test.go b/pkg/apis/messaging/v1alpha1/channel_types_test.go index 4d77b22959..df006a8616 100644 --- a/pkg/apis/messaging/v1alpha1/channel_types_test.go +++ b/pkg/apis/messaging/v1alpha1/channel_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" "k8s.io/apimachinery/pkg/runtime/schema" @@ -67,3 +69,27 @@ func TestChannelIdentityStatus(t *testing.T) { 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) + } +} From 70fef5f96efb2a7694e65e79387001f219410647 Mon Sep 17 00:00:00 2001 From: grac3gao Date: Thu, 26 Mar 2020 12:10:31 -0700 Subject: [PATCH 4/8] fix typo --- pkg/apis/duck/v1alpha1/identity_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/duck/v1alpha1/identity_types.go b/pkg/apis/duck/v1alpha1/identity_types.go index 5febc811c7..668f4ddd10 100644 --- a/pkg/apis/duck/v1alpha1/identity_types.go +++ b/pkg/apis/duck/v1alpha1/identity_types.go @@ -22,7 +22,7 @@ import ( duckv1 "knative.dev/pkg/apis/duck/v1" ) -// PubSub is an Implementable "duck type". +// Identity is an Implementable "duck type". var _ duck.Implementable = (*Identity)(nil) // +genduck @@ -93,7 +93,7 @@ func (*Identity) GetListType() runtime.Object { // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object -// IdentityList is a list of PubSub resources +// IdentityList is a list of Identity resources type IdentityList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata"` From 7a74487d47729ef87c87a3fd002ad1f0d82d1d5f Mon Sep 17 00:00:00 2001 From: grac3gao Date: Thu, 26 Mar 2020 13:56:24 -0700 Subject: [PATCH 5/8] update code --- pkg/reconciler/events/auditlogs/auditlogs_test.go | 2 +- pkg/reconciler/events/pubsub/pubsub_test.go | 2 +- pkg/reconciler/events/scheduler/scheduler_test.go | 2 +- pkg/reconciler/events/storage/storage_test.go | 2 +- pkg/reconciler/messaging/channel/channel_test.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/reconciler/events/auditlogs/auditlogs_test.go b/pkg/reconciler/events/auditlogs/auditlogs_test.go index 55d9493870..1535d3c30c 100644 --- a/pkg/reconciler/events/auditlogs/auditlogs_test.go +++ b/pkg/reconciler/events/auditlogs/auditlogs_test.go @@ -124,7 +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 issue https://github.com/google/knative-gcp/issues/657 gets solved. +// 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 diff --git a/pkg/reconciler/events/pubsub/pubsub_test.go b/pkg/reconciler/events/pubsub/pubsub_test.go index 77bb44ff8d..01ed7e8861 100644 --- a/pkg/reconciler/events/pubsub/pubsub_test.go +++ b/pkg/reconciler/events/pubsub/pubsub_test.go @@ -125,7 +125,7 @@ func newSink() *unstructured.Unstructured { } } -// TODO add a unit test for successfully creating a k8s service account, after issue issue https://github.com/google/knative-gcp/issues/657 gets solved. +// 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 diff --git a/pkg/reconciler/events/scheduler/scheduler_test.go b/pkg/reconciler/events/scheduler/scheduler_test.go index b9eaa1262a..ffe3bc205d 100644 --- a/pkg/reconciler/events/scheduler/scheduler_test.go +++ b/pkg/reconciler/events/scheduler/scheduler_test.go @@ -141,7 +141,7 @@ func newSink() *unstructured.Unstructured { } } -// TODO add a unit test for successfully creating a k8s service account, after issue issue https://github.com/google/knative-gcp/issues/657 gets solved. +// 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 diff --git a/pkg/reconciler/events/storage/storage_test.go b/pkg/reconciler/events/storage/storage_test.go index cda478ad2c..2c49a6ed80 100644 --- a/pkg/reconciler/events/storage/storage_test.go +++ b/pkg/reconciler/events/storage/storage_test.go @@ -140,7 +140,7 @@ func newSink() *unstructured.Unstructured { } } -// TODO add a unit test for successfully creating a k8s service account, after issue issue https://github.com/google/knative-gcp/issues/657 gets solved. +// 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 diff --git a/pkg/reconciler/messaging/channel/channel_test.go b/pkg/reconciler/messaging/channel/channel_test.go index 5b6ecd2222..7db4da1409 100644 --- a/pkg/reconciler/messaging/channel/channel_test.go +++ b/pkg/reconciler/messaging/channel/channel_test.go @@ -101,7 +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 issue https://github.com/google/knative-gcp/issues/657 gets solved. +// 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", From 41a1e18d4179b7ae9508d5401632bd1fe9f0b5f5 Mon Sep 17 00:00:00 2001 From: grac3gao Date: Thu, 26 Mar 2020 16:22:13 -0700 Subject: [PATCH 6/8] update code --- pkg/apis/duck/v1alpha1/pubsub_types.go | 5 +++++ pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go | 6 ++++++ pkg/apis/events/v1alpha1/cloudauditlogssource_types.go | 3 ++- pkg/apis/events/v1alpha1/cloudpubsubsource_types.go | 3 ++- pkg/apis/events/v1alpha1/cloudschedulersource_types.go | 3 ++- pkg/apis/events/v1alpha1/cloudstoragesource_types.go | 3 ++- pkg/apis/messaging/v1alpha1/channel_types.go | 4 +++- pkg/duck/identifiable.go | 4 +++- 8 files changed, 25 insertions(+), 6 deletions(-) diff --git a/pkg/apis/duck/v1alpha1/pubsub_types.go b/pkg/apis/duck/v1alpha1/pubsub_types.go index 494c1417d1..4fdc2674da 100644 --- a/pkg/apis/duck/v1alpha1/pubsub_types.go +++ b/pkg/apis/duck/v1alpha1/pubsub_types.go @@ -74,6 +74,11 @@ type PubSubStatus struct { // +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 ProjectID string `json:"projectId,omitempty"` diff --git a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go index de9f59766d..3d9ec5486f 100644 --- a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go @@ -24,6 +24,7 @@ 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. @@ -213,6 +214,11 @@ func (in *PubSubStatus) DeepCopyInto(out *PubSubStatus) { *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_types.go b/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go index 640fdc9227..00fdc88c14 100644 --- a/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go +++ b/pkg/apis/events/v1alpha1/cloudauditlogssource_types.go @@ -108,11 +108,12 @@ func (*CloudAuditLogsSource) GetGroupVersionKind() schema.GroupVersionKind { } // Methods for identifiable interface. - +// IdentitySpec returns the IdentitySpec portion of the Spec. func (s *CloudAuditLogsSource) IdentitySpec() *duckv1alpha1.IdentitySpec { return &s.Spec.IdentitySpec } +// IdentityStatus returns the IdentityStatus portion of the Status. func (s *CloudAuditLogsSource) IdentityStatus() *duckv1alpha1.IdentityStatus { return &s.Status.IdentityStatus } diff --git a/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go b/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go index fae8dbaf9b..9599e769d8 100644 --- a/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go +++ b/pkg/apis/events/v1alpha1/cloudpubsubsource_types.go @@ -147,11 +147,12 @@ func (s *CloudPubSubSource) GetGroupVersionKind() schema.GroupVersionKind { } // Methods for identifiable interface. - +// IdentitySpec returns the IdentitySpec portion of the Spec. func (s *CloudPubSubSource) IdentitySpec() *duckv1alpha1.IdentitySpec { return &s.Spec.IdentitySpec } +// IdentityStatus returns the IdentityStatus portion of the Status. func (s *CloudPubSubSource) IdentityStatus() *duckv1alpha1.IdentityStatus { return &s.Status.IdentityStatus } diff --git a/pkg/apis/events/v1alpha1/cloudschedulersource_types.go b/pkg/apis/events/v1alpha1/cloudschedulersource_types.go index ae7913217b..cd8681f210 100644 --- a/pkg/apis/events/v1alpha1/cloudschedulersource_types.go +++ b/pkg/apis/events/v1alpha1/cloudschedulersource_types.go @@ -111,11 +111,12 @@ func (scheduler *CloudSchedulerSource) GetGroupVersionKind() schema.GroupVersion } // Methods for identifiable interface - +// IdentitySpec returns the IdentitySpec portion of the Spec. func (s *CloudSchedulerSource) IdentitySpec() *duckv1alpha1.IdentitySpec { return &s.Spec.IdentitySpec } +// IdentityStatus returns the IdentityStatus portion of the Status. func (s *CloudSchedulerSource) IdentityStatus() *duckv1alpha1.IdentityStatus { return &s.Status.IdentityStatus } diff --git a/pkg/apis/events/v1alpha1/cloudstoragesource_types.go b/pkg/apis/events/v1alpha1/cloudstoragesource_types.go index 4011708c67..010a5fa15f 100644 --- a/pkg/apis/events/v1alpha1/cloudstoragesource_types.go +++ b/pkg/apis/events/v1alpha1/cloudstoragesource_types.go @@ -127,11 +127,12 @@ func (storage *CloudStorageSource) GetGroupVersionKind() schema.GroupVersionKind } // Methods for identifiable interface. - +// IdentitySpec returns the IdentitySpec portion of the Spec. func (s *CloudStorageSource) IdentitySpec() *duckv1alpha1.IdentitySpec { return &s.Spec.IdentitySpec } +// IdentityStatus returns the IdentityStatus portion of the Status. func (s *CloudStorageSource) IdentityStatus() *duckv1alpha1.IdentityStatus { return &s.Status.IdentityStatus } diff --git a/pkg/apis/messaging/v1alpha1/channel_types.go b/pkg/apis/messaging/v1alpha1/channel_types.go index fbacbff4bd..afc60d3c8a 100644 --- a/pkg/apis/messaging/v1alpha1/channel_types.go +++ b/pkg/apis/messaging/v1alpha1/channel_types.go @@ -123,15 +123,17 @@ type ChannelStatus struct { } // 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 } diff --git a/pkg/duck/identifiable.go b/pkg/duck/identifiable.go index 8dc4a3c412..371fc9ae08 100644 --- a/pkg/duck/identifiable.go +++ b/pkg/duck/identifiable.go @@ -25,8 +25,10 @@ import ( type Identifiable interface { kmeta.OwnerRefable - // GetIdentity returns identifiable's identity. + // 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 } From 2bb315a4db838746be9a123d0ef984a88235fee3 Mon Sep 17 00:00:00 2001 From: grac3gao Date: Thu, 26 Mar 2020 17:53:29 -0700 Subject: [PATCH 7/8] update code --- pkg/apis/duck/v1alpha1/identity_types.go | 49 --------------- pkg/apis/duck/v1alpha1/identity_types_test.go | 34 ----------- .../duck/v1alpha1/zz_generated.deepcopy.go | 61 ------------------- .../ducks/duck/v1alpha1/identity/fake/fake.go | 30 --------- .../ducks/duck/v1alpha1/identity/identity.go | 60 ------------------ 5 files changed, 234 deletions(-) delete mode 100644 pkg/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go delete mode 100644 pkg/client/injection/ducks/duck/v1alpha1/identity/identity.go diff --git a/pkg/apis/duck/v1alpha1/identity_types.go b/pkg/apis/duck/v1alpha1/identity_types.go index 668f4ddd10..7f5c13fab7 100644 --- a/pkg/apis/duck/v1alpha1/identity_types.go +++ b/pkg/apis/duck/v1alpha1/identity_types.go @@ -14,28 +14,10 @@ limitations under the License. package v1alpha1 import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "knative.dev/pkg/apis" - "knative.dev/pkg/apis/duck" duckv1 "knative.dev/pkg/apis/duck/v1" ) -// Identity is an Implementable "duck type". -var _ duck.Implementable = (*Identity)(nil) - -// +genduck -// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object - -type Identity struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec IdentitySpec `json:"spec"` - Status IdentityStatus `json:"status"` -} - 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. @@ -69,34 +51,3 @@ func (ss *IdentityStatus) IsReady() bool { } return false } - -var ( - // Verify Identity resources meet duck contracts. - _ duck.Populatable = (*Identity)(nil) - _ apis.Listable = (*Identity)(nil) -) - -// GetFullType implements duck.Implementable -func (*Identity) GetFullType() duck.Populatable { - return &Identity{} -} - -// Populate implements duck.Populatable -func (s *Identity) Populate() { - s.Spec.ServiceAccount = "" -} - -// GetListType implements apis.Listable -func (*Identity) GetListType() runtime.Object { - return &IdentityList{} -} - -// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object - -// IdentityList is a list of Identity resources -type IdentityList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata"` - - Items []Identity `json:"items"` -} diff --git a/pkg/apis/duck/v1alpha1/identity_types_test.go b/pkg/apis/duck/v1alpha1/identity_types_test.go index 6910d9366b..33fb55e17c 100644 --- a/pkg/apis/duck/v1alpha1/identity_types_test.go +++ b/pkg/apis/duck/v1alpha1/identity_types_test.go @@ -21,40 +21,6 @@ import ( "testing" ) -func TestIdentityGetListType(t *testing.T) { - c := &Identity{} - switch c.GetListType().(type) { - case *IdentityList: - // expected - default: - t.Errorf("expected GetListType to return *ChannelableList, got %T", c.GetListType()) - } -} - -func TestIdentityPopulate(t *testing.T) { - got := &Identity{} - want := &Identity{ - Spec: IdentitySpec{}, - } - - got.Populate() - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("Unexpected difference (-want, +got): %v", diff) - } -} - -func TestGetFullType(t *testing.T) { - got := &Identity{} - want := &Identity{} - - got.GetFullType() - - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("Unexpected difference (-want, +got): %v", diff) - } -} - func TestIsReady(t *testing.T) { status := &IdentityStatus{} want := false diff --git a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go index 3d9ec5486f..1be4d2e5a5 100644 --- a/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/duck/v1alpha1/zz_generated.deepcopy.go @@ -27,67 +27,6 @@ import ( 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 *Identity) DeepCopyInto(out *Identity) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec - in.Status.DeepCopyInto(&out.Status) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Identity. -func (in *Identity) DeepCopy() *Identity { - if in == nil { - return nil - } - out := new(Identity) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *Identity) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *IdentityList) DeepCopyInto(out *IdentityList) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ListMeta.DeepCopyInto(&out.ListMeta) - if in.Items != nil { - in, out := &in.Items, &out.Items - *out = make([]Identity, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IdentityList. -func (in *IdentityList) DeepCopy() *IdentityList { - if in == nil { - return nil - } - out := new(IdentityList) - in.DeepCopyInto(out) - return out -} - -// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -func (in *IdentityList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil -} - // 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 diff --git a/pkg/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go b/pkg/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go deleted file mode 100644 index 7d866c948e..0000000000 --- a/pkg/client/injection/ducks/duck/v1alpha1/identity/fake/fake.go +++ /dev/null @@ -1,30 +0,0 @@ -/* -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. -*/ - -// Code generated by injection-gen. DO NOT EDIT. - -package fake - -import ( - identity "github.com/google/knative-gcp/pkg/client/injection/ducks/duck/v1alpha1/identity" - injection "knative.dev/pkg/injection" -) - -var Get = identity.Get - -func init() { - injection.Fake.RegisterDuck(identity.WithDuck) -} diff --git a/pkg/client/injection/ducks/duck/v1alpha1/identity/identity.go b/pkg/client/injection/ducks/duck/v1alpha1/identity/identity.go deleted file mode 100644 index b3cf4dc84e..0000000000 --- a/pkg/client/injection/ducks/duck/v1alpha1/identity/identity.go +++ /dev/null @@ -1,60 +0,0 @@ -/* -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. -*/ - -// Code generated by injection-gen. DO NOT EDIT. - -package identity - -import ( - context "context" - - v1alpha1 "github.com/google/knative-gcp/pkg/apis/duck/v1alpha1" - duck "knative.dev/pkg/apis/duck" - controller "knative.dev/pkg/controller" - injection "knative.dev/pkg/injection" - dynamicclient "knative.dev/pkg/injection/clients/dynamicclient" - logging "knative.dev/pkg/logging" -) - -func init() { - injection.Default.RegisterDuck(WithDuck) -} - -// Key is used for associating the Informer inside the context.Context. -type Key struct{} - -func WithDuck(ctx context.Context) context.Context { - dc := dynamicclient.Get(ctx) - dif := &duck.CachedInformerFactory{ - Delegate: &duck.TypedInformerFactory{ - Client: dc, - Type: (&v1alpha1.Identity{}).GetFullType(), - ResyncPeriod: controller.GetResyncPeriod(ctx), - StopChannel: ctx.Done(), - }, - } - return context.WithValue(ctx, Key{}, dif) -} - -// Get extracts the typed informer from the context. -func Get(ctx context.Context) duck.InformerFactory { - untyped := ctx.Value(Key{}) - if untyped == nil { - logging.FromContext(ctx).Panic( - "Unable to fetch knative.dev/pkg/apis/duck.InformerFactory from context.") - } - return untyped.(duck.InformerFactory) -} From addbe0b7f62e28c434ef090969d8f55ee5f14e43 Mon Sep 17 00:00:00 2001 From: grac3gao Date: Fri, 27 Mar 2020 11:31:53 -0700 Subject: [PATCH 8/8] update code --- pkg/apis/duck/v1alpha1/identity_lifecycle.go | 3 +++ pkg/apis/duck/v1alpha1/identity_types.go | 1 + pkg/reconciler/identity/reconciler.go | 1 + 3 files changed, 5 insertions(+) diff --git a/pkg/apis/duck/v1alpha1/identity_lifecycle.go b/pkg/apis/duck/v1alpha1/identity_lifecycle.go index 92a4fcd810..62cdbda1b4 100644 --- a/pkg/apis/duck/v1alpha1/identity_lifecycle.go +++ b/pkg/apis/duck/v1alpha1/identity_lifecycle.go @@ -28,5 +28,8 @@ func (s *IdentityStatus) MarkWorkloadIdentityNotConfigured(cs *apis.ConditionSet 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_types.go b/pkg/apis/duck/v1alpha1/identity_types.go index 7f5c13fab7..6515692011 100644 --- a/pkg/apis/duck/v1alpha1/identity_types.go +++ b/pkg/apis/duck/v1alpha1/identity_types.go @@ -22,6 +22,7 @@ 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"` } diff --git a/pkg/reconciler/identity/reconciler.go b/pkg/reconciler/identity/reconciler.go index 80967e788e..2e8f13408e 100644 --- a/pkg/reconciler/identity/reconciler.go +++ b/pkg/reconciler/identity/reconciler.go @@ -66,6 +66,7 @@ func (i *Identity) ReconcileWorkloadIdentity(ctx context.Context, projectID stri 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)