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

Include cluster name in k8s Service Account name for Workload Identity #950

Merged
merged 29 commits into from
May 12, 2020
Merged

Include cluster name in k8s Service Account name for Workload Identity #950

merged 29 commits into from
May 12, 2020

Conversation

grac3gao-zz
Copy link
Contributor

@grac3gao-zz grac3gao-zz commented Apr 27, 2020 •
edited by grantr
Loading

Fixes #715
Fixes #735

Proposed Changes

  • Use an annotation cluster-name to include metadata.clusterName, and leave it as empty if the user is not running in GKE
  • Rename the KSA for the Workload Identity

Release Note

Docs

@grac3gao-zz grac3gao-zz requested a review from grantr April 27, 2020 17:01
@googlebot googlebot added the cla: yes (override cla status due to multiple authors bug) label Apr 27, 2020
@grac3gao-zz grac3gao-zz mentioned this pull request Apr 29, 2020
Closed
@grac3gao-zz
Copy link
Contributor Author

/test pull-google-knative-gcp-unit-tests

@grac3gao-zz grac3gao-zz changed the title Include cluster name in k8s Service Account name for Workload Identity [WIP]Include cluster name in k8s Service Account name for Workload Identity Apr 30, 2020
@grac3gao
Merge remote-tracking branch 'upstream/master' into cluster
9ab2cc9 
# Conflicts:
#	pkg/apis/pubsub/v1alpha1/pullsubscription_defaults.go
#	pkg/apis/pubsub/v1alpha1/pullsubscription_defaults_test.go
#	pkg/apis/pubsub/v1alpha1/pullsubscription_validation.go
#	pkg/apis/pubsub/v1alpha1/topic_defaults.go
#	pkg/apis/pubsub/v1alpha1/topic_defaults_test.go
#	pkg/apis/pubsub/v1alpha1/topic_validation.go
@grac3gao-zz grac3gao-zz changed the title [WIP]Include cluster name in k8s Service Account name for Workload Identity Include cluster name in k8s Service Account name for Workload Identity May 1, 2020
@grac3gao-zz
Copy link
Contributor Author

ready to review

grantr
grantr approved these changes May 11, 2020
View reviewed changes
Copy link
Contributor

@grantr grantr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grac3gao, grantr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@grantr grantr mentioned this pull request May 11, 2020
Closed
@grantr
Copy link
Contributor

grantr commented May 11, 2020

Fixes #735

grantr
grantr reviewed May 11, 2020
View reviewed changes
Copy link
Contributor

@grantr grantr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@grac3gao-zz
Copy link
Contributor Author

/test pull-google-knative-gcp-unit-tests

2 similar comments
@grac3gao-zz
Copy link
Contributor Author

/test pull-google-knative-gcp-unit-tests

@grac3gao-zz
Copy link
Contributor Author

/test pull-google-knative-gcp-unit-tests

@grantr
Copy link
Contributor

grantr commented May 12, 2020

/lgtm

@Harwayne
Copy link
Contributor

/retest

@knative-metrics-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-google-knative-gcp-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/duck/v1alpha1/annotations.go 93.2% 94.4% 1.3
pkg/apis/events/v1alpha1/cloudauditlogssource_validation.go 88.9% 94.7% 5.8
pkg/apis/events/v1alpha1/cloudbuildsource_validation.go 88.2% 88.9% 0.7
pkg/apis/events/v1alpha1/cloudpubsubsource_validation.go 93.1% 93.3% 0.2
pkg/apis/events/v1alpha1/cloudschedulersource_validation.go 95.2% 100.0% 4.8
pkg/apis/events/v1alpha1/cloudstoragesource_defaults.go 60.0% 100.0% 40.0
pkg/apis/events/v1alpha1/cloudstoragesource_validation.go 94.1% 100.0% 5.9
pkg/apis/intevents/v1alpha1/pullsubscription_defaults.go Do not exist 100.0%
pkg/apis/intevents/v1alpha1/pullsubscription_validation.go Do not exist 88.6%
pkg/apis/intevents/v1alpha1/topic_defaults.go Do not exist 100.0%
pkg/apis/intevents/v1alpha1/topic_validation.go Do not exist 100.0%
pkg/apis/messaging/v1alpha1/channel_defaults.go 100.0% 88.9% -11.1
pkg/apis/messaging/v1alpha1/channel_validation.go 84.2% 90.0% 5.8
pkg/reconciler/broker/broker.go 55.9% 53.7% -2.2
pkg/reconciler/broker/controller.go 83.3% 81.6% -1.8
pkg/reconciler/broker/trigger.go 58.4% 55.9% -2.6
pkg/reconciler/helper.go Do not exist 92.1%
pkg/reconciler/identity/reconciler.go 72.7% 68.4% -4.3
pkg/reconciler/intevents/pullsubscription/resources/receive_adapter.go Do not exist 87.5%
pkg/reconciler/intevents/reconciler.go Do not exist 73.9%
pkg/reconciler/intevents/resources/topic.go Do not exist 100.0%
pkg/reconciler/intevents/topic/resources/publisher.go Do not exist 85.7%
pkg/reconciler/intevents/topic/topic.go Do not exist 69.6%
pkg/reconciler/messaging/channel/channel.go 79.6% 79.9% 0.3
pkg/reconciler/pubsub/pullsubscription/resources/receive_adapter.go 87.5% 87.9% 0.4
pkg/reconciler/pubsub/reconciler.go 73.7% 73.9% 0.2
pkg/reconciler/pubsub/topic/resources/publisher.go 85.7% 86.7% 1.0
pkg/reconciler/pubsub/topic/topic.go 69.6% 69.9% 0.3
pkg/utils/metadata.go 0.0% 100.0% 100.0

@grantr
Copy link
Contributor

grantr commented May 12, 2020

/retest

@grantr
Copy link
Contributor

grantr commented May 12, 2020

/lgtm

@grac3gao-zz
Copy link
Contributor Author

/test pull-google-knative-gcp-integration-tests

@knative-prow-robot knative-prow-robot merged commit 3fe97fd into google:master May 12, 2020
@grac3gao-zz grac3gao-zz mentioned this pull request May 12, 2020
Closed
@grac3gao-zz grac3gao-zz deleted the cluster branch May 20, 2020 23:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@grantr grantr grantr approved these changes

@ericlem ericlem Awaiting requested review from ericlem

@Harwayne Harwayne Awaiting requested review from Harwayne

Assignees

@grantr grantr

Labels
approved cla: yes (override cla status due to multiple authors bug) lgtm size/XXL
Projects
None yet
Milestone
No milestone
7 participants
@grac3gao-zz @knative-prow-robot @grantr @Harwayne @knative-metrics-robot @googlebot @grac3gao