Fix the update validation error while stopping an idling workspace. #15689
Conversation
is anonymous. This will skip the check in the scheduled jobs, where we are in control of the data and will not affect the user initiated creations or updates where an anonymous user would be rejected anyway. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
metlos
requested review from
amisevsk,
l0rd,
nickboldt,
rhopp and
sleshchenko
as code owners
| @@ -76,7 +77,14 @@ public void validate(Map<String, String> attributes) throws ValidationException | |||
| + "')"); | |||
| } | |||
|
|
|||
| namespaceFactoryProvider.get().checkIfNamespaceIsAllowed(namespace); | |||
There was a problem hiding this comment.
Might be wrong, but according to the comment below it looks like idling is broken in the upstream also, no ? (anonymous subject for scheduled jobs like idling is used in the similar manner in the upstream)
There was a problem hiding this comment.
if this is the case it is just not clear for me why this error was not caught by the pre-release tests..
There was a problem hiding this comment.
@dmytro-ndp @rhopp could you please take a look? would be great to understand if the pre-release testing actually run the idling tests or this use-case is not tested atm?
There was a problem hiding this comment.
We talked about this and as far as I understood, this bug doesn't exhibit itself because in the pre-release tests, we use a namespace default that doesn't have <username> or <userid> placeholder.
There was a problem hiding this comment.
@metlos could you point me to the code which checks the namespace part and placeholders / ? For now I do not fully understand how idling works and it looks like it works differently depending on the namespace strategies
There was a problem hiding this comment.
This is the method used during workspace update to check that the namespace contained in the attributes is valid: https://github.com/eclipse/che/blob/b2b6f5f488ea523bac0a9313f0f65a23ac98a16a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java#L139
Note that the current version of code in this branch calls this during the update only during a situation that I am not sure can happen even.
…idling Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
...g/eclipse/che/workspace/infrastructure/kubernetes/K8sInfraNamespaceWsAttributeValidator.java
Outdated
Show resolved
Hide resolved
based on the suggestion by @sleshche. Also updated the javadocs to clarify the responsibilities of the validation methods. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
|
Can one of the admins verify this patch? |
che-bot
added
status/code-review
|
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
|
✅ E2E Happy path tests succeed 🎉 See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) |
|
E2E tests of Eclipse Che Multiuser on OCP has been successful:
|
che-bot
removed
the
status/code-review
…15689) Rework the namespace validation based on the suggestion by @sleshche. We now don't do full validation of update using the `validate` method but rather rely on `validateUpdate` to do the equivalent job. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
…15689) Rework the namespace validation based on the suggestion by @sleshche. We now don't do full validation of update using the `validate` method but rather rely on `validateUpdate` to do the equivalent job. Signed-off-by: Lukas Krejci <lkrejci@redhat.com> [TS_SELENIUM] Create a method for expanding project tree according to the provided path (#15754) Signed-off-by: Ihor Okhrimenko <iokhrime@redhat.com> merge with upstream change echo message put commands into functions increase load page timeout add necessary function reduce timeout
What does this PR do?
Do not perform the workspace namespace validation if the current user is anonymous.
This will skip the check in the scheduled jobs, where we are
in control of the data and will not affect the user initiated creations
or updates where an anonymous user would be rejected anyway.
What issues does this PR fix or reference?
#15667