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

Fix resource listing #171

Merged
merged 4 commits into from
Oct 2, 2024
Merged

Fix resource listing #171

merged 4 commits into from
Oct 2, 2024

Conversation

thirdeyenick
Copy link
Contributor

@thirdeyenick thirdeyenick commented Oct 1, 2024

This fixes the not working resource listing over all projects. We now search for the users projects and search in them for the corresponding resource if --all-projects/-A was given. As this required quite some changes in test setups (projects have to be around when testing the --all-projects functionality), this PR also extends the internal test framework which can be used to setup a fake clientset. All manual creations of fake clientsets have been replaced by the test framework.

The main changes of this PR are in internal/test/client.go and get/get.go. All the other changes are test framework adaptions or cleanups of existing tests.

This fixes an issue with listing of resources. To list resources we need to first gather all the projects of a user. Afterwards we can search for the corresponding type in each project.
Copy link
Contributor

@ctrox ctrox left a comment

Choose a reason for hiding this comment

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

One negative side-effect that I noticed is that now Niners with full permissions can't list objects in all namespaces anymore. But I think we can't really have both so we might just need to live with it.

@thirdeyenick
Copy link
Contributor Author

One negative side-effect that I noticed is that now Niners with full permissions can't list objects in all namespaces anymore. But I think we can't really have both so we might just need to live with it.

Yeah, true this a negative side effect. This could be changed by adding an option, but I'd say it is rather uncommon to have something like an 'admin' option in such a tool.

To overcome some import cycles, this commit moves the kubeconfig extension type and also functions related to the kubeconfig into its own package.
@thirdeyenick thirdeyenick merged commit 9e3c4a3 into main Oct 2, 2024
2 checks passed
@thirdeyenick thirdeyenick deleted the fix-listing branch October 2, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants