:sparkles: Wire up Service Account by theishshah · Pull Request #1038 · operator-framework/operator-controller

theishshah · 2024-07-11T18:12:16Z

Description

#973 Add functionality to use Service Account

netlify · 2024-07-11T18:12:34Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`d1eb632`
🔍 Latest deploy log	https://app.netlify.com/sites/olmv1/deploys/66995b9c2bb9d100086b47f6
😎 Deploy Preview	https://deploy-preview-1038--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2024-07-11T18:18:46Z

Codecov Report

Attention: Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.

Project coverage is 72.25%. Comparing base (aff11ee) to head (d1eb632).
Report is 3 commits behind head on main.

Files	Patch %	Lines
cmd/manager/main.go	63.15%	4 Missing and 3 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1038 +/- ##
==========================================
- Coverage 72.50% 72.25% -0.26% 
==========================================
Files 32 32 Lines 1884 1903 +19 ==========================================
+ Hits 1366 1375 +9 - Misses 383 389 +6 - Partials 135 139 +4

Flag	Coverage Δ
e2e	`56.00% <63.15%> (+0.78%)`	⬆️
unit	`44.50% <0.00%> (-0.45%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

joelanford · 2024-07-16T21:18:33Z

 	cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
 helmclient.StorageNamespaceMapper(installNamespaceMapper),
 helmclient.ClientNamespaceMapper(installNamespaceMapper),
+ helmclient.RestConfigMapper(restConfigMapper),


If this lands soon, all good. But just wanted to let you know that I'm working on making REST config mapping handle storage and client interactions separately. The intent is that we would use our own service account for storage (since that is an implementation detail of our controller) and we would only use the ClusterExtension's service account for the helm client's REST config.
See:
✨ Add support for a chunked release storage driver helm-operator-plugins#350
⚠️ helm: use chunking release driver in systemNamespace #1057

Thanks for the heads up! I spoke with @theishshah and I think we will focus on getting this in using the soon-to-be-deprecated RestConfigMapper option and as a follow up address this (if the helm-operator-plugins changes land before this gets in)

Signed-off-by: Ish Shah <ishah@redhat.com>

tmshort

/lgtm

everettraven · 2024-07-18T20:35:20Z

+//+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create
+//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get


For other reviewers, the serviceaccounts/token permissions were required to get tokens for a provided SA and the customresourcedefinitions permissions are for the CRD Upgrade Safety preflight checks

This reverts commit 95b9f0d.

* add logic to return service account Signed-off-by: Ish Shah <ishah@redhat.com> * update permissions and anon token Signed-off-by: Ish Shah <ishah@redhat.com> * updated role yaml Signed-off-by: Ish Shah <ishah@redhat.com> * clean up imports Signed-off-by: Ish Shah <ishah@redhat.com> * update e2e tests Signed-off-by: Ish Shah <ishah@redhat.com> * fix lint Signed-off-by: Ish Shah <ishah@redhat.com> * extension developer test fixed Signed-off-by: Ish Shah <ishah@redhat.com> * stand up sa for upgrade test Signed-off-by: Ish Shah <ishah@redhat.com> * fixed upgrade test Signed-off-by: Ish Shah <ishah@redhat.com> * linting for extension test Signed-off-by: Ish Shah <ishah@redhat.com> --------- Signed-off-by: Ish Shah <ishah@redhat.com>

openshift-ciBot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 11, 2024

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 13, 2024

joelanford reviewed Jul 16, 2024
View reviewed changes

theishshah force-pushed the wireup-sa branch from ddf5369 to cc6c725Compare July 17, 2024 17:34

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 17, 2024

theishshah changed the title ~~[WIP] Wire up Service Account~~Wire up Service AccountJul 18, 2024

Ish Shah added 9 commits July 18, 2024 11:10

add logic to return service account
d99940d
Signed-off-by: Ish Shah <ishah@redhat.com>

update permissions and anon token
11d8f90
Signed-off-by: Ish Shah <ishah@redhat.com>

updated role yaml
dbe5705
Signed-off-by: Ish Shah <ishah@redhat.com>

clean up imports
fbcf4c8
Signed-off-by: Ish Shah <ishah@redhat.com>

update e2e tests
0e5dc71
Signed-off-by: Ish Shah <ishah@redhat.com>

fix lint
182648a
Signed-off-by: Ish Shah <ishah@redhat.com>

extension developer test fixed
09efb2a
Signed-off-by: Ish Shah <ishah@redhat.com>

stand up sa for upgrade test
c04d504
Signed-off-by: Ish Shah <ishah@redhat.com>

fixed upgrade test
0715393
Signed-off-by: Ish Shah <ishah@redhat.com>

theishshah force-pushed the wireup-sa branch from fe43cb4 to 0715393Compare July 18, 2024 18:10

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2024

theishshah changed the title ~~Wire up Service Account~~✨ Wire up Service AccountJul 18, 2024

theishshah marked this pull request as ready for review July 18, 2024 18:11

theishshah requested a review from a team as a code owner July 18, 2024 18:11

openshift-ciBot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 18, 2024

linting for extension test
d1eb632
Signed-off-by: Ish Shah <ishah@redhat.com>

tmshort approved these changes Jul 18, 2024
View reviewed changes

openshift-ciBot assigned tmshort Jul 18, 2024

openshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jul 18, 2024

everettraven reviewed Jul 18, 2024
View reviewed changes

everettraven approved these changes Jul 18, 2024
View reviewed changes

everettraven added this pull request to the merge queue Jul 18, 2024

Merged via the queue into operator-framework:main with commit 95b9f0dJul 18, 2024

joelanford mentioned this pull request Jul 19, 2024
Wire up installation of content using a provided ServiceAccount #973
Closed

joelanford added a commit to joelanford/operator-controller that referenced this pull request Jul 19, 2024
Revert ":sparkles: Wire up Service Account (operator-framework#1038)"
bb4171f
This reverts commit 95b9f0d.

joelanford mentioned this pull request Jul 19, 2024
🐛 Revert "✨ Wire up Service Account (#1038)" #1072
Closed
4 tasks

github-merge-queueBot pushed a commit that referenced this pull request Jul 19, 2024
Revert ":sparkles: Wire up Service Account (#1038)" (#1072)
193281f
This reverts commit 95b9f0d.

This was referenced Jul 19, 2024
🐛 add cluster-admin permissions to o-c SA temporarily #1073
Merged
✨ wire up ServiceAccount based caching layer #1074
Merged

skattoju mentioned this pull request Sep 9, 2024
🌱 Descriptive Upgrade Constraint Policy constants #1233
Merged

skattoju mentioned this pull request Sep 25, 2024
✨ Remove unpacked status #1309
Merged
4 tasks

Navigation Menu

Uh oh!

✨ Wire up Service Account - #1038

✨ Wire up Service Account#1038
everettraven merged 10 commits into
operator-framework:mainfrom
theishshah:wireup-sa

theishshah commented Jul 11, 2024

Uh oh!

netlifyBot commented Jul 11, 2024•
edited
Loading

Uh oh!

codecovBot commented Jul 11, 2024•
edited
Loading

Uh oh!

joelanfordJul 16, 2024

Uh oh!

everettravenJul 17, 2024

Uh oh!

tmshort left a comment

Uh oh!

everettravenJul 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		//+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create
		//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get

Uh oh!

Conversation

theishshah commented Jul 11, 2024

Description

Uh oh!

netlifyBot commented Jul 11, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

codecovBot commented Jul 11, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joelanfordJul 16, 2024

Choose a reason for hiding this comment

Uh oh!

everettravenJul 17, 2024

Choose a reason for hiding this comment

Uh oh!

tmshort left a comment

Choose a reason for hiding this comment

Uh oh!

everettravenJul 18, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

netlifyBot commented Jul 11, 2024•
edited
Loading

codecovBot commented Jul 11, 2024•
edited
Loading