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

Support auto generation of Parallel identity service account and expose in AuthStatus #7225

Closed
creydr opened this issue Sep 5, 2023 · 16 comments · Fixed by #7373
Closed
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@creydr
Copy link
Member

creydr commented Sep 5, 2023

Problem

As the Eventing OIDC feature track describes, the AuthStatus is meant to provide the generated service account name in the resource status.

After #7173 is done, we should:

@creydr creydr added kind/TBD Parked issue that required triaging/revisit in a near future. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed kind/TBD Parked issue that required triaging/revisit in a near future. labels Sep 5, 2023
@aliok
Copy link
Member

aliok commented Sep 12, 2023

hi @creydr

I see you've added help-wanted label to this issue. If you think this is a good good-first-issue, can you:

  • Comment with /good-first-issue?
  • Add clear instructions (including code pointers) about how this issue can be resolved?

@creydr
Copy link
Member Author

creydr commented Sep 29, 2023

#7299 and #7316 show how this was done for the Trigger and could help to get started with this.

/good-first-issue

@knative-prow
Copy link

knative-prow bot commented Sep 29, 2023

@creydr:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

#7299 and #7316 show how this was done for the Trigger and could help to get started with this.

/good-first-issue

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.

@knative-prow knative-prow bot added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Sep 29, 2023
@prakrit55
Copy link
Contributor

/assign

@creydr
Copy link
Member Author

creydr commented Oct 5, 2023

Hello @pradnyavmw,
Thanks for picking this 🎉
Do you have a rough ETA, when a PR will be ready for review?

@prakrit55
Copy link
Contributor

Hello @pradnyavmw, Thanks for picking this 🎉 Do you have a rough ETA, when a PR will be ready for review?

hey @creydr, I will get a pr here soon

@creydr
Copy link
Member Author

creydr commented Oct 5, 2023

hey @creydr, I will get a pr here soon

Awesome! Thanks for letting us know

@prakrit55
Copy link
Contributor

hey @creydr, I was going through the ticket, can you specify the parallel lifecycle file where I will make the changes on sa

@creydr
Copy link
Member Author

creydr commented Oct 10, 2023

Hello @prakrit55,
the reconcilation of a parallel is defined in pkg/reconciler/parallel/parallel.go. There you need to create the SA and update the Parallels status.

The setup of the reconciler is done in pkg/reconciler/parallel/controller.go. In this file you need to setup the informers and configmap watchers.

Does this help you to get started?

@prakrit55
Copy link
Contributor

Great.....thanks, with the referenced pr I m ready to implement it

@prakrit55
Copy link
Contributor

Hey @creydr, does the parallel generate subscriptions, IMO we need to create the SA in generated the identities acc. to the feature track and we supply it through ObjectReference

So, we create the SA in ChannelTemplate
Pls, rectify me if I am wrong.

@creydr
Copy link
Member Author

creydr commented Oct 13, 2023

Hey @creydr, does the parallel generate subscriptions, IMO we need to create the SA in generated the identities acc. to the feature track and we supply it through ObjectReference

So, we create the SA in ChannelTemplate Pls, rectify me if I am wrong.

Hey @prakrit55,
not sure if I can follow you completely. The SA generation for a Subscriptions and Channels are done in other issues. In this issue, we should handle only the one for the Parallel.

@prakrit55
Copy link
Contributor

@creydr , I got ur point so, should I create it in parallelspec in parallel_types.go ??

@creydr
Copy link
Member Author

creydr commented Oct 13, 2023

parallel_types.go contains only the type definition of the parallel. The reconciling of a Parallel resource is handled in the reconciler (parallel.go). There you would need to create the SA.
The configuration of some config watchers (needed to track the feature config map) is done in the reconciler setup in controller.go.

There are a couple of PRs, which do the same for other resource types. If the referenced PRs from the description don't help you can also check on #7338 (which is WIP/not merged yet though)

@prakrit55
Copy link
Contributor

hey @creydr, should I configure the parallel_test.go and controllers_test.go too ??

@creydr
Copy link
Member Author

creydr commented Oct 17, 2023

hey @creydr, should I configure the parallel_test.go and controllers_test.go too ??

Hello @prakrit55,
that would be great, to have some tests for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants