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
kubevirt: don't set template namespace #4701
kubevirt: don't set template namespace #4701
Conversation
@@ -126,7 +126,6 @@ export const createVM = async (params: CreateVMParams) => { | |||
// ProcessedTemplates endpoint will reject the request if user cannot post to the namespace | |||
// common-templates are stored in openshift namespace, default user can read but cannot post | |||
template | |||
.setNamespace(namespace) |
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.
are you sure the user has the right to create the template in the openshift namespace? Hence the comment above.
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.
We are only concerned about the vm pointer to the parent template right? It should probably be enough to restore the old namespace just before the line 148 https://github.com/openshift/console/pull/4701/files#diff-e76664ae71de5c525cc5ed6e2f799809R148
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.
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.
are you sure the user has the right to create the template in the openshift namespace?
@suomiy Correct !!
tested with unprivileged user, can't process a template in "openshift" namespace.
It should probably be enough to restore the old namespace
Will try this, thanks 👍
Signed-off-by: yaacov <kobi.zamir@gmail.com>
e8d9a55
to
9439d63
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: suomiy, yaacov The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test e2e-gcp-console |
/test e2e-gcp-console |
2 similar comments
/test e2e-gcp-console |
/test e2e-gcp-console |
/retest Please review the full test history for this PR and help us cut down flakes. |
/test analyze |
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/test e2e-gcp-console |
/test analyze |
/test e2e-gcp-console |
1 similar comment
/test e2e-gcp-console |
/test e2e-gcp-console |
1 similar comment
/test e2e-gcp-console |
When creating a VM using base template we change the template namespace.
If template namespace is not the current namespace ( e.g. when using base templtates ) we will get an error, and the VM will fail to create.
Screenshots:
After:
Before:
Settings: