-
Notifications
You must be signed in to change notification settings - Fork 339
Fix documentation security issue #665
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
base: master
Are you sure you want to change the base?
Conversation
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.
Can you also add a test covering this change?
Please run make generate generate-website-docs generate-artifacthub-artifacts to generate the templates and docs.
|
@TIJMacLean did you miss running |
|
@JaydipGabani I hadn't - I've tried to do that all now but as a non-developer, I've been struggling to get all the dependencies and versions working correctly. I've tried again with the make commands and having moved the changes into the src directory instead, but I could get the unit tests to run correctly on my system. So hopefully they pass here! |
|
@JaydipGabani Afternoon - is there any update on whether this is more in line with what you were expecting? |
* updating GK and dep versions Signed-off-by: Jaydip Gabani <[email protected]> * fixing link for kubectl binary Signed-off-by: Jaydip Gabani <[email protected]> * fixing tests where APIs no longer exists Signed-off-by: Jaydip Gabani <[email protected]> * updating k8s version and fixing tests Signed-off-by: Jaydip Gabani <[email protected]> --------- Signed-off-by: Jaydip Gabani <[email protected]>
d895e5e to
7441716
Compare
Signed-off-by: Tom MacLean <[email protected]>
Signed-off-by: Tom MacLean <[email protected]>
Signed-off-by: Tom MacLean <[email protected]>
|
Didn't realise I had to be the one to sign off the messages - assumed that was an admin right. I also got the unit tests running so hopefully this is now good to go! |
|
@TIJMacLean please run |
Signed-off-by: Tom MacLean <[email protected]>
|
Ok - did that again which should be it (fingers crossed) |
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.
please fix nits, lgtm otherwise
| "name": "exempt", | ||
| "image": "exempt:testing", | ||
| }] | ||
| }] |
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.
nit: add a new line
| args: | ||
| - "run" | ||
| - "--server" | ||
| - "--addr=localhost:8080" No newline at end of file |
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.
nit: add a new line
| args: | ||
| - "run" | ||
| - "--server" | ||
| - "--addr=localhost:8080" No newline at end of file |
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.
nit: add a new line
What this PR does / why we need it:
Fixes a security issue where the "latest" tag could be deployed even when it was disallowed, by using the format image:port/repo for a container image rather than the expected image/repo:tag. image:port/repo passed the contains ":" check, and defaults to pulling the latest
Which issue(s) does this PR fix (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes a report through the security email
Special notes for your reviewer: