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.

Sign up for GitHub

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

Jump to bottom

feat: add user impersonation to all commands #549

Merged
merged 15 commits into from
Aug 22, 2024
Merged

feat: add user impersonation to all commands #549

merged 15 commits into from
Aug 22, 2024

Conversation

logan-bobo
Copy link
Contributor

@logan-bobo logan-bobo commented Aug 5, 2024 •
edited
Loading

What this PR does / why we need it:

Fixes # #547

Working configuration after adding the --as flag for user impersonation.

./bin/kubectl-kuttl test --config ... --as admin
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at:...
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 30 seconds for each step
    harness.go:375: testsuite: ...
=== RUN   kuttl/harness
=== RUN   kuttl/harness/00-check-deploy
=== PAUSE kuttl/harness/00-check-deploy
=== CONT  kuttl/harness/00-check-deploy
    logger.go:42: 19:45:28 | 00-check-deploy | Creating namespace: kuttl-test-engaged-cockatoo
    logger.go:42: 19:45:28 | 00-check-deploy/0- | starting test step 0-
    logger.go:42: 19:45:29 | 00-check-deploy/0- | test step completed 0-
    logger.go:42: 19:45:29 | 00-check-deploy | skipping kubernetes event logging
    logger.go:42: 19:45:29 | 00-check-deploy | Deleting namespace: kuttl-test-engaged-cockatoo
=== NAME  kuttl
    harness.go:407: run tests finished
    harness.go:515: cleaning up
    harness.go:572: removing temp folder: ""
--- PASS: kuttl (7.71s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/00-check-deploy (7.16s)
PASS

Without having user impersonation

❯ ./bin/kubectl-kuttl test --config ...
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at:...
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 30 seconds for each step
    harness.go:375: testsuite:...
=== RUN   kuttl/harness
=== RUN   kuttl/harness/00-check-deploy
=== PAUSE kuttl/harness/00-check-deploy
=== CONT  kuttl/harness/00-check-deploy
    logger.go:42: 19:47:44 | 00-check-deploy | Creating namespace: kuttl-test-awaited-dingo
    case.go:357: namespaces is forbidden: User "" cannot create resource "namespaces" in API group "" at the cluster scope
    logger.go:42: 19:47:44 | 00-check-deploy | Deleting namespace: kuttl-test-awaited-dingo
    case.go:117: namespaces "kuttl-test-awaited-dingo" is forbidden: User "" cannot delete resource "namespaces" in API group "" in the namespace "kuttl-test-awaited-dingo"
=== NAME  kuttl
    harness.go:407: run tests finished
    harness.go:515: cleaning up
    harness.go:572: removing temp folder: ""
--- FAIL: kuttl (0.84s)
    --- FAIL: kuttl/harness (0.00s)
        --- FAIL: kuttl/harness/00-check-deploy (0.31s)
FAIL

@logan-bobo logan-bobo marked this pull request as draft August 5, 2024 17:39
logan-bobo
logan-bobo commented Aug 5, 2024
View reviewed changes
pkg/kuttlctl/cmd/root.go Outdated Show resolved Hide resolved
@logan-bobo logan-bobo force-pushed the logan-bobo/enable-user-impersonation branch from 9132d76 to 10227a0 Compare August 8, 2024 20:44
@logan-bobo
Copy link
Contributor Author

I have managed to pass the as flag down to the two types of client that are created Client and DiscoveryClient please let me know what you think of the approach and I will add the remaining commands.

@logan-bobo logan-bobo changed the title feat: add k8s generic cli options as flags to main cmd feat: add user impersonation to all commands Aug 9, 2024
porridge
porridge reviewed Aug 9, 2024
View reviewed changes
Copy link
Member

@porridge porridge left a comment

Choose a reason for hiding this comment

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

I'm afraid plumbing this and related flags all the way this way is not going to be very sustainable 😟 ).

Can we try keeping the Client signature unchanged? I'm AFK so cannot easily check everything, but see an idea in an inline comment.

pkg/test/assert.go Outdated Show resolved Hide resolved
@logan-bobo logan-bobo force-pushed the logan-bobo/enable-user-impersonation branch 2 times, most recently from 6274230 to 497795f Compare August 9, 2024 18:01
@logan-bobo logan-bobo force-pushed the logan-bobo/enable-user-impersonation branch from 0057ada to aa58e1d Compare August 20, 2024 11:59
@logan-bobo logan-bobo force-pushed the logan-bobo/enable-user-impersonation branch 2 times, most recently from 34d97b5 to 09252ed Compare August 21, 2024 06:34
logan-bobo
logan-bobo commented Aug 21, 2024
View reviewed changes
pkg/kuttlctl/cmd/root.go Outdated Show resolved Hide resolved
porridge
porridge reviewed Aug 21, 2024
View reviewed changes
Copy link
Member

@porridge porridge left a comment

Choose a reason for hiding this comment

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

Excellent, this looks really neat!

Can you please:

  • also change the call in Harness.Config
  • make sure you update docs/
  • since this is not very testable without specific cluster configuration, please copy-paste into PR description some terminal output showing how this fails without the option and works with it

pkg/kuttlctl/cmd/root.go Outdated Show resolved Hide resolved
@logan-bobo logan-bobo force-pushed the logan-bobo/enable-user-impersonation branch 3 times, most recently from e8bd4ae to 4fc8bc2 Compare August 21, 2024 17:47
@logan-bobo logan-bobo marked this pull request as ready for review August 21, 2024 19:09
porridge
porridge approved these changes Aug 22, 2024
View reviewed changes
Copy link
Member

@porridge porridge left a comment

Choose a reason for hiding this comment

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

Excellent ❤️
Just a few more nitpicks..

docs/cli.md Outdated Show resolved Hide resolved
pkg/test/assert.go Outdated Show resolved Hide resolved
pkg/test/assert.go Outdated Show resolved Hide resolved
pkg/test/assert.go Outdated Show resolved Hide resolved
pkg/test/assert.go Outdated Show resolved Hide resolved
pkg/k8s/main.go Outdated Show resolved Hide resolved
pkg/k8s/main.go Outdated Show resolved Hide resolved
@porridge
Copy link
Member

Ah, 💩
I applied my nitpicks to save some back-and-forth and forgot that this will break the DCO check 🤦🏻
And now I cannot fix it easily because I cannot push to your fork. Please help! 😅

porridge and others added 6 commits August 22, 2024 08:52
@porridge @logan-bobo
Make is possible to build with Go 1.23. (kudobuilder#548)
1ab1d97 
Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
add k8s generic cli options as flags to main cmd
2a4a9ba 
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
feat: pass impersonation user to REST config construction
a531789 
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
feat: split out impersonation into its own package
6f2febf 
Signed-off-by: Logan Cox <mail@logan-cox.com>

Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
feat: assign imperisonation user via distinct package
54c1448 
Signed-off-by: Logan Cox <mail@logan-cox.com>

Signed-off-by: Logan Cox <mail@logan-cox.com>
@dependabot @logan-bobo
Bump the kubernetes group with 8 updates (kudobuilder#550)
4e79d86 
Bumps the kubernetes group with 8 updates:

| Package | From | To |
| --- | --- | --- |
| [k8s.io/api](https://github.com/kubernetes/api) | `0.30.3` | `0.31.0` |
| [k8s.io/apiextensions-apiserver](https://github.com/kubernetes/apiextensions-apiserver) | `0.30.3` | `0.31.0` |
| [k8s.io/apimachinery](https://github.com/kubernetes/apimachinery) | `0.30.3` | `0.31.0` |
| [k8s.io/client-go](https://github.com/kubernetes/client-go) | `0.30.3` | `0.31.0` |
| [k8s.io/code-generator](https://github.com/kubernetes/code-generator) | `0.30.3` | `0.31.0` |
| [sigs.k8s.io/controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) | `0.18.4` | `0.19.0` |
| [sigs.k8s.io/controller-tools](https://github.com/kubernetes-sigs/controller-tools) | `0.15.0` | `0.16.1` |
| [sigs.k8s.io/kind](https://github.com/kubernetes-sigs/kind) | `0.23.0` | `0.24.0` |

Updates `k8s.io/api` from 0.30.3 to 0.31.0
- [Commits](kubernetes/api@v0.30.3...v0.31.0)

Updates `k8s.io/apiextensions-apiserver` from 0.30.3 to 0.31.0
- [Release notes](https://github.com/kubernetes/apiextensions-apiserver/releases)
- [Commits](kubernetes/apiextensions-apiserver@v0.30.3...v0.31.0)

Updates `k8s.io/apimachinery` from 0.30.3 to 0.31.0
- [Commits](kubernetes/apimachinery@v0.30.3...v0.31.0)

Updates `k8s.io/client-go` from 0.30.3 to 0.31.0
- [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md)
- [Commits](kubernetes/client-go@v0.30.3...v0.31.0)

Updates `k8s.io/code-generator` from 0.30.3 to 0.31.0
- [Commits](kubernetes/code-generator@v0.30.3...v0.31.0)

Updates `sigs.k8s.io/controller-runtime` from 0.18.4 to 0.19.0
- [Release notes](https://github.com/kubernetes-sigs/controller-runtime/releases)
- [Changelog](https://github.com/kubernetes-sigs/controller-runtime/blob/main/RELEASE.md)
- [Commits](kubernetes-sigs/controller-runtime@v0.18.4...v0.19.0)

Updates `sigs.k8s.io/controller-tools` from 0.15.0 to 0.16.1
- [Release notes](https://github.com/kubernetes-sigs/controller-tools/releases)
- [Changelog](https://github.com/kubernetes-sigs/controller-tools/blob/main/envtest-releases.yaml)
- [Commits](kubernetes-sigs/controller-tools@v0.15.0...v0.16.1)

Updates `sigs.k8s.io/kind` from 0.23.0 to 0.24.0
- [Release notes](https://github.com/kubernetes-sigs/kind/releases)
- [Commits](kubernetes-sigs/kind@v0.23.0...v0.24.0)

---
updated-dependencies:
- dependency-name: k8s.io/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/apiextensions-apiserver
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/apimachinery
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/client-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/code-generator
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: sigs.k8s.io/controller-runtime
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: sigs.k8s.io/controller-tools
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: sigs.k8s.io/kind
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Logan Cox <mail@logan-cox.com>
dependabot bot and others added 6 commits August 22, 2024 08:52
@dependabot @logan-bobo
Bump github.com/docker/docker (kudobuilder#551)
64d46cb 
Bumps [github.com/docker/docker](https://github.com/docker/docker) from 27.1.1+incompatible to 27.1.2+incompatible.
- [Release notes](https://github.com/docker/docker/releases)
- [Commits](moby/moby@v27.1.1...v27.1.2)

---
updated-dependencies:
- dependency-name: github.com/docker/docker
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Logan Cox <mail@logan-cox.com>
@dependabot @logan-bobo
Bump the kubernetes group with 8 updates (kudobuilder#550)
12b5e93 
Bumps the kubernetes group with 8 updates:

| Package | From | To |
| --- | --- | --- |
| [k8s.io/api](https://github.com/kubernetes/api) | `0.30.3` | `0.31.0` |
| [k8s.io/apiextensions-apiserver](https://github.com/kubernetes/apiextensions-apiserver) | `0.30.3` | `0.31.0` |
| [k8s.io/apimachinery](https://github.com/kubernetes/apimachinery) | `0.30.3` | `0.31.0` |
| [k8s.io/client-go](https://github.com/kubernetes/client-go) | `0.30.3` | `0.31.0` |
| [k8s.io/code-generator](https://github.com/kubernetes/code-generator) | `0.30.3` | `0.31.0` |
| [sigs.k8s.io/controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) | `0.18.4` | `0.19.0` |
| [sigs.k8s.io/controller-tools](https://github.com/kubernetes-sigs/controller-tools) | `0.15.0` | `0.16.1` |
| [sigs.k8s.io/kind](https://github.com/kubernetes-sigs/kind) | `0.23.0` | `0.24.0` |

Updates `k8s.io/api` from 0.30.3 to 0.31.0
- [Commits](kubernetes/api@v0.30.3...v0.31.0)

Updates `k8s.io/apiextensions-apiserver` from 0.30.3 to 0.31.0
- [Release notes](https://github.com/kubernetes/apiextensions-apiserver/releases)
- [Commits](kubernetes/apiextensions-apiserver@v0.30.3...v0.31.0)

Updates `k8s.io/apimachinery` from 0.30.3 to 0.31.0
- [Commits](kubernetes/apimachinery@v0.30.3...v0.31.0)

Updates `k8s.io/client-go` from 0.30.3 to 0.31.0
- [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md)
- [Commits](kubernetes/client-go@v0.30.3...v0.31.0)

Updates `k8s.io/code-generator` from 0.30.3 to 0.31.0
- [Commits](kubernetes/code-generator@v0.30.3...v0.31.0)

Updates `sigs.k8s.io/controller-runtime` from 0.18.4 to 0.19.0
- [Release notes](https://github.com/kubernetes-sigs/controller-runtime/releases)
- [Changelog](https://github.com/kubernetes-sigs/controller-runtime/blob/main/RELEASE.md)
- [Commits](kubernetes-sigs/controller-runtime@v0.18.4...v0.19.0)

Updates `sigs.k8s.io/controller-tools` from 0.15.0 to 0.16.1
- [Release notes](https://github.com/kubernetes-sigs/controller-tools/releases)
- [Changelog](https://github.com/kubernetes-sigs/controller-tools/blob/main/envtest-releases.yaml)
- [Commits](kubernetes-sigs/controller-tools@v0.15.0...v0.16.1)

Updates `sigs.k8s.io/kind` from 0.23.0 to 0.24.0
- [Release notes](https://github.com/kubernetes-sigs/kind/releases)
- [Commits](kubernetes-sigs/kind@v0.23.0...v0.24.0)

---
updated-dependencies:
- dependency-name: k8s.io/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/apiextensions-apiserver
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/apimachinery
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/client-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: k8s.io/code-generator
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: sigs.k8s.io/controller-runtime
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: sigs.k8s.io/controller-tools
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
- dependency-name: sigs.k8s.io/kind
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: kubernetes
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
feat: add new GetConfig command and inject ImpersonateAs variable
f092edd 
Signed-off-by: Logan Cox <mail@logan-cox.com>

Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
Update pkg/kuttlctl/cmd/root.go
1448b80 
Signed-off-by: Author Name <authoremail@example.com>
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
feat: fmt
9b643ce 
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
doc: add docs for user impersonation
d57eb56 
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo logan-bobo force-pushed the logan-bobo/enable-user-impersonation branch from 832d572 to d57eb56 Compare August 22, 2024 07:52
@logan-bobo
Copy link
Contributor Author

@porridge I will apply these and sign them

@logan-bobo
feat: nit
2a549b1 
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
feat: nit
1645853 
Signed-off-by: Logan Cox <mail@logan-cox.com>
@logan-bobo
Copy link
Contributor Author

@porridge all of your comments have been added to the latest commit 🤝

@porridge porridge merged commit 4ea17e9 into kudobuilder:main Aug 22, 2024
5 checks passed
@porridge porridge mentioned this pull request Aug 22, 2024
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@porridge porridge porridge approved these changes

@kensipe kensipe Awaiting requested review from kensipe kensipe is a code owner

Assignees
No one assigned
Labels
release/highlight
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@logan-bobo @porridge