-
Notifications
You must be signed in to change notification settings - Fork 214
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
add ability to audit a single workload #368
Conversation
Codecov Report
@@ Coverage Diff @@
## master #368 +/- ##
==========================================
- Coverage 53.55% 48.27% -5.28%
==========================================
Files 12 12
Lines 732 812 +80
==========================================
Hits 392 392
- Misses 285 365 +80
Partials 55 55
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good except that inconsistency. I wonder if we could infer version somehow.... I haven't found a good way to mock out the dynamic client yet, maybe stuff like that needs to stay in the realm of e2e tests?
|
||
parts := strings.Split(workload, "/") | ||
if len(parts) != 4 { | ||
return nil, fmt.Errorf("Invalid workload identifier %s. Should be in format namespace/kind/version/name, e.g. nginx-ingress/Deployment.apps/v1/default-backend", workload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the help text you ask for 3 pieces (namespace/kind/name) instead of including version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
79cdb9d
to
38a9ba6
Compare
Sadly I haven't been able to test this - I can't get DynamicClient to work properly in test mode. Any ideas would be appreciated.