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

Update default credential and rename method #1214

Merged
merged 5 commits into from
Jun 5, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions pkg/apis/duck/v1alpha1/pubsub_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@ func (s *PubSubSpec) SetPubSubDefaults(ctx context.Context) {
logging.FromContext(ctx).Error("Failed to get the GCPAuthDefaults")
return
}
if s.ServiceAccountName == "" {
if s.ServiceAccountName == "" &&
(s.Secret == nil || equality.Semantic.DeepEqual(s.Secret, &corev1.SecretKeySelector{})) {
s.ServiceAccountName = ad.KSA(apis.ParentMeta(ctx).Namespace)
}
if s.Secret == nil || equality.Semantic.DeepEqual(s.Secret, &corev1.SecretKeySelector{}) {
s.Secret = ad.Secret(apis.ParentMeta(ctx).Namespace)
}
}
14 changes: 13 additions & 1 deletion pkg/apis/duck/v1alpha1/pubsub_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1alpha1

import (
"context"
"testing"

gcpauthtesthelper "github.com/google/knative-gcp/pkg/apis/configs/gcpauth/testhelper"
Expand All @@ -29,6 +30,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
testCases := map[string]struct {
orig *PubSubSpec
expected *PubSubSpec
ctx context.Context
}{
"missing defaults": {
orig: &PubSubSpec{},
Expand All @@ -40,6 +42,12 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"missing default GCP Auth ctx": {
orig: &PubSubSpec{},
expected: &PubSubSpec{},
ctx: context.Background(),
},
"empty defaults": {
orig: &PubSubSpec{
Expand All @@ -53,6 +61,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"secret exists same key": {
orig: &PubSubSpec{
Expand All @@ -71,6 +80,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"secret exists same name": {
orig: &PubSubSpec{
Expand All @@ -89,6 +99,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "different-key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"secret exists all different": {
orig: &PubSubSpec{
Expand All @@ -107,11 +118,12 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "different-key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
}
for n, tc := range testCases {
t.Run(n, func(t *testing.T) {
tc.orig.SetPubSubDefaults(gcpauthtesthelper.ContextWithDefaults())
tc.orig.SetPubSubDefaults(tc.ctx)
if diff := cmp.Diff(tc.expected, tc.orig); diff != "" {
t.Errorf("Unexpected differences (-want +got): %v", diff)
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/duck/v1beta1/pubsub_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@ func (s *PubSubSpec) SetPubSubDefaults(ctx context.Context) {
logging.FromContext(ctx).Error("Failed to get the GCPAuthDefaults")
return
}
if s.ServiceAccountName == "" {
if s.ServiceAccountName == "" &&
(s.Secret == nil || equality.Semantic.DeepEqual(s.Secret, &corev1.SecretKeySelector{})) {
s.ServiceAccountName = ad.KSA(apis.ParentMeta(ctx).Namespace)
}
if s.Secret == nil || equality.Semantic.DeepEqual(s.Secret, &corev1.SecretKeySelector{}) {
s.Secret = ad.Secret(apis.ParentMeta(ctx).Namespace)
}
}
14 changes: 13 additions & 1 deletion pkg/apis/duck/v1beta1/pubsub_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1beta1

import (
"context"
"testing"

gcpauthtesthelper "github.com/google/knative-gcp/pkg/apis/configs/gcpauth/testhelper"
Expand All @@ -29,6 +30,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
testCases := map[string]struct {
orig *PubSubSpec
expected *PubSubSpec
ctx context.Context
}{
"missing defaults": {
orig: &PubSubSpec{},
Expand All @@ -40,6 +42,12 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"missing default GCP Auth ctx": {
orig: &PubSubSpec{},
expected: &PubSubSpec{},
ctx: context.Background(),
},
"empty defaults": {
orig: &PubSubSpec{
Expand All @@ -53,6 +61,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"secret exists same key": {
orig: &PubSubSpec{
Expand All @@ -71,6 +80,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"secret exists same name": {
orig: &PubSubSpec{
Expand All @@ -89,6 +99,7 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "different-key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"secret exists all different": {
orig: &PubSubSpec{
Expand All @@ -107,11 +118,12 @@ func TestPubSubSpec_SetPubSubDefaults(t *testing.T) {
Key: "different-key.json",
},
},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
}
for n, tc := range testCases {
t.Run(n, func(t *testing.T) {
tc.orig.SetPubSubDefaults(gcpauthtesthelper.ContextWithDefaults())
tc.orig.SetPubSubDefaults(tc.ctx)
if diff := cmp.Diff(tc.expected, tc.orig); diff != "" {
t.Errorf("Unexpected differences (-want +got): %v", diff)
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/intevents/v1alpha1/topic_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ func (ts *TopicSpec) SetDefaults(ctx context.Context) {
logging.FromContext(ctx).Error("Failed to get the GCPAuthDefaults")
return
}
if ts.ServiceAccountName == "" {
if ts.ServiceAccountName == "" &&
(ts.Secret == nil || equality.Semantic.DeepEqual(ts.Secret, &corev1.SecretKeySelector{})) {
ts.ServiceAccountName = ad.KSA(apis.ParentMeta(ctx).Namespace)
}
if ts.Secret == nil || equality.Semantic.DeepEqual(ts.Secret, &corev1.SecretKeySelector{}) {
ts.Secret = ad.Secret(apis.ParentMeta(ctx).Namespace)
}
}
67 changes: 43 additions & 24 deletions pkg/apis/intevents/v1alpha1/topic_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1alpha1

import (
"context"
"testing"

gcpauthtesthelper "github.com/google/knative-gcp/pkg/apis/configs/gcpauth/testhelper"
Expand All @@ -30,32 +31,50 @@ import (
)

func TestTopicDefaults(t *testing.T) {
want := &Topic{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
},
},
Spec: TopicSpec{
PropagationPolicy: TopicPolicyCreateNoDelete,
Secret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "google-cloud-key",
testCases := map[string]struct {
want *Topic
got *Topic
ctx context.Context
}{
"with GCP Auth": {
want: &Topic{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
},
},
Key: "key.json",
},
}}

got := &Topic{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
Spec: TopicSpec{
PropagationPolicy: TopicPolicyCreateNoDelete,
Secret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "google-cloud-key",
},
Key: "key.json",
},
}},
got: &Topic{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
},
},
Spec: TopicSpec{}},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"without GCP Auth": {
want: &Topic{Spec: TopicSpec{
PropagationPolicy: TopicPolicyCreateNoDelete},
},
got: &Topic{},
ctx: context.Background(),
},
Spec: TopicSpec{}}
got.SetDefaults(gcpauthtesthelper.ContextWithDefaults())

if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("failed to get expected (-want, +got) = %v", diff)
}
for n, tc := range testCases {
t.Run(n, func(t *testing.T) {
tc.got.SetDefaults(tc.ctx)
if diff := cmp.Diff(tc.want, tc.got); diff != "" {
t.Errorf("Unexpected differences (-want +got): %v", diff)
}
})
}
}
5 changes: 2 additions & 3 deletions pkg/apis/intevents/v1beta1/topic_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ func (ts *TopicSpec) SetDefaults(ctx context.Context) {
logging.FromContext(ctx).Error("Failed to get the GCPAuthDefaults")
return
}
if ts.ServiceAccountName == "" {
if ts.ServiceAccountName == "" &&
(ts.Secret == nil || equality.Semantic.DeepEqual(ts.Secret, &corev1.SecretKeySelector{})) {
ts.ServiceAccountName = ad.KSA(apis.ParentMeta(ctx).Namespace)
}
if ts.Secret == nil || equality.Semantic.DeepEqual(ts.Secret, &corev1.SecretKeySelector{}) {
ts.Secret = ad.Secret(apis.ParentMeta(ctx).Namespace)
}
}
45 changes: 32 additions & 13 deletions pkg/apis/intevents/v1beta1/topic_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1beta1

import (
"context"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -25,20 +26,38 @@ import (
)

func TestTopicDefaults(t *testing.T) {
want := &Topic{Spec: TopicSpec{
PropagationPolicy: TopicPolicyCreateNoDelete,
Secret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "google-cloud-key",
testCases := map[string]struct {
want *Topic
got *Topic
ctx context.Context
}{
"with GCP Auth": {
want: &Topic{Spec: TopicSpec{
PropagationPolicy: TopicPolicyCreateNoDelete,
Secret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "google-cloud-key",
},
Key: "key.json",
},
}},
got: &Topic{Spec: TopicSpec{}},
ctx: gcpauthtesthelper.ContextWithDefaults(),
},
"without GCP Auth": {
want: &Topic{Spec: TopicSpec{
PropagationPolicy: TopicPolicyCreateNoDelete},
},
Key: "key.json",
got: &Topic{},
ctx: context.Background(),
},
}}

got := &Topic{Spec: TopicSpec{}}
got.SetDefaults(gcpauthtesthelper.ContextWithDefaults())

if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("failed to get expected (-want, +got) = %v", diff)
}
for n, tc := range testCases {
t.Run(n, func(t *testing.T) {
tc.got.SetDefaults(tc.ctx)
if diff := cmp.Diff(tc.want, tc.got); diff != "" {
t.Errorf("Unexpected differences (-want +got): %v", diff)
}
})
}
}
8 changes: 4 additions & 4 deletions pkg/reconciler/events/auditlogs/auditlogs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func TestAllCases(t *testing.T) {
WithCloudAuditLogsSourceAnnotations(map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
}),
WithCloudAuditLogsSourceDefaultAuthorization(),
WithCloudAuditLogsSourceDefaultGCPAuth(),
),
NewTopic(sourceName, testNS,
WithTopicSpec(inteventsv1alpha1.TopicSpec{
Expand All @@ -424,7 +424,7 @@ func TestAllCases(t *testing.T) {
WithTopicReady(testTopicID),
WithTopicAddress(testTopicURI),
WithTopicProjectID(testProject),
WithTopicDefaultAuthorization(),
WithTopicDefaultGCPAuth(),
),
},
Key: testNS + "/" + sourceName,
Expand All @@ -439,7 +439,7 @@ func TestAllCases(t *testing.T) {
WithCloudAuditLogsSourceAnnotations(map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
}),
WithCloudAuditLogsSourceDefaultAuthorization(),
WithCloudAuditLogsSourceDefaultGCPAuth(),
WithCloudAuditLogsSourcePullSubscriptionUnknown("PullSubscriptionNotConfigured", failedToReconcilePullSubscriptionMsg),
),
}},
Expand All @@ -462,7 +462,7 @@ func TestAllCases(t *testing.T) {
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
}),
WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{sourceOwnerRef(sourceName, sourceUID)}),
WithPullSubscriptionDefaultAuthorization(),
WithPullSubscriptionDefaultGCPAuth(),
),
},
WantPatches: []clientgotesting.PatchActionImpl{
Expand Down
6 changes: 3 additions & 3 deletions pkg/reconciler/events/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func TestAllCases(t *testing.T) {
WithCloudBuildSourceAnnotations(map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
}),
WithCloudBuildSourceDefaultAuthorization(),
WithCloudBuildSourceDefaultGCPAuth(),
),
newSink(),
},
Expand All @@ -182,7 +182,7 @@ func TestAllCases(t *testing.T) {
WithCloudBuildSourceAnnotations(map[string]string{
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
}),
WithCloudBuildSourceDefaultAuthorization(),
WithCloudBuildSourceDefaultGCPAuth(),
WithCloudBuildSourcePullSubscriptionUnknown("PullSubscriptionNotConfigured", "PullSubscription has not yet been reconciled"),
),
}},
Expand All @@ -207,7 +207,7 @@ func TestAllCases(t *testing.T) {
duckv1alpha1.ClusterNameAnnotation: testingMetadataClient.FakeClusterName,
}),
WithPullSubscriptionOwnerReferences([]metav1.OwnerReference{ownerRef()}),
WithPullSubscriptionDefaultAuthorization(),
WithPullSubscriptionDefaultGCPAuth(),
),
},
WantPatches: []clientgotesting.PatchActionImpl{
Expand Down
Loading