Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add service account resource #588

Closed

Conversation

creydr
Copy link
Contributor

@creydr creydr commented Sep 26, 2023

Changes

  • 🎁 Add helpers to work with service accounts

/kind enhancement

@knative-prow
Copy link

knative-prow bot commented Sep 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr

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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 26, 2023
@creydr
Copy link
Contributor Author

creydr commented Sep 26, 2023

/assign @pierDipi

@knative-prow knative-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 26, 2023
Comment on lines +52 to +54
// WaitForBeingAvailable waits until the given service account was created by
// some 3rd party
func WaitForBeingAvailable(name string, timing ...time.Duration) feature.StepFn {
Copy link
Member

@pierDipi pierDipi Sep 26, 2023

Choose a reason for hiding this comment

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

This is to very that a given resource like "trigger" has created a service account?

Copy link
Member

@pierDipi pierDipi Sep 26, 2023

Choose a reason for hiding this comment

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

Ideally, we don't enter in such implementation details in E2E tests, the contract and therefore the E2E behavior we need to verify is that "the subscriber will receive a JWT token emitted by the k8s configured issuer", the SA is the means we use for that but I think we should verify that the subscriber is receiving such tokens not that a SA is created (that's an implementation detail)

Copy link
Contributor Author

@creydr creydr Sep 26, 2023

Choose a reason for hiding this comment

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

This is to very that a given resource like "trigger" has created a service account?

Yes.

Ideally, we don't enter in such implementation details in E2E tests

hmmm. I see your point, but how would you check then, that e.g. the trigger reconciler really creates the OIDC SA?

Copy link
Member

@pierDipi pierDipi Sep 26, 2023

Choose a reason for hiding this comment

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

I would verify that the token you receive in the eventshub subscriber is what you expect (you need to do that anyway since that actually tests everything E2E, data plane included)

Copy link
Member

Choose a reason for hiding this comment

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

basically, we don't check "The SA is created" but "the token with the SA identity is received" and that's transitively testing that the SA exists

@creydr
Copy link
Contributor Author

creydr commented Sep 26, 2023

As discussed, we will close this for now until we are clearer how to proceed for the eventhub

@creydr creydr closed this Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/enhancement size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants