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

[Draft] PoC for Workload Identity status condition #703

Closed
wants to merge 1 commit into from

Conversation

nachocano
Copy link
Member

Proposed Changes

  • @grac3gao this is an example of how it might look like. We had a bunch of unnecessary stuff. Probably we have more, but I haven't had the time to clean it up. Take a look and let me know if it makes sense.
  • No need to do it, I'd rather have the e2e tests working first. But if you have cycles, might be worth giving it a shot.

Release Note


Docs

/hold won't submit this, it's just for demonstration purposes...

@googlebot googlebot added the cla: yes (override cla status due to multiple authors bug) label Mar 25, 2020
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nachocano

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

@nachocano
Copy link
Member Author

/assign @grac3gao

// 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"`
Copy link
Member Author

Choose a reason for hiding this comment

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

@grac3gao I think you were right. I think we should call this something different, otherwise it might be confusing with a k8s service account. I'm inclined to call it gsa not to be too verbose with googleServiceAccount, but I don't have strong opinion...
This is lower priority, but something to keep in mind, because it changes a bunch of places..

Comment on lines +31 to +40
// +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"`
}
Copy link
Member Author

Choose a reason for hiding this comment

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

this duck is not even needed.

@knative-prow-robot
Copy link
Contributor

@nachocano: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-google-knative-gcp-unit-tests 01c59b4 link /test pull-google-knative-gcp-unit-tests
pull-google-knative-gcp-build-tests 01c59b4 link /test pull-google-knative-gcp-build-tests
pull-google-knative-gcp-integration-tests 01c59b4 link /test pull-google-knative-gcp-integration-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved cla: yes (override cla status due to multiple authors bug) do-not-merge/hold size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants