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

Fixes requiring namespace when namespace variable is not set #30

Merged
merged 3 commits into from
Aug 14, 2020

Conversation

mkgrei
Copy link
Contributor

@mkgrei mkgrei commented Aug 13, 2020

#29

Fixing namespace not set error, when kind is not ServiceAccount.

In this fix, when kind is Group, the summary for role will not be shown even if present.
In the current implementation, no summary will be shown for this corner case.

To allow summary for Group with role, the following line should be modified to add Group.
https://github.com/Ladicle/kubectl-rolesum/blob/master/cmd/cmd.go#L111
This PR do not fix this problem.

test.sh Outdated
@@ -1,7 +1,7 @@
#!/bin/bash -eu

echo; echo "Creating ServiceAccount..."
kubectl create sa test-user
kubectl create sa test-user --dry-run -o yaml | kubectl apply -f -
Copy link
Owner

Choose a reason for hiding this comment

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

The --dry-run option has been deprecated since v1.18+, so could you replace them with --dry-run=client?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought I saw some deprecation error...
Fixed them.
Thank you.

Copy link
Owner

@Ladicle Ladicle left a comment

Choose a reason for hiding this comment

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

LGTM

@Ladicle Ladicle merged commit 29d66b1 into Ladicle:master Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants