Fail workspace validation if config contains both ws.next plugins and installers#11392
Fail workspace validation if config contains both ws.next plugins and installers#11392amisevsk merged 2 commits intoeclipse-che:masterfrom
Conversation
Raise error if workspace config contains both plugins and installers, preventing user from creating workspace with both, which can result in confusion. Signed-off-by: Angel Misevski <amisevsk@redhat.com>
amisevsk
added
the
kind/task
|
ci-test |
benoitf
added
the
status/code-review
amisevsk
changed the title
|
Results of automated E2E tests of Eclipse Che Multiuser on OCP: |
| .map(MachineConfig::getInstallers) | ||
| .flatMap(List::stream)) | ||
| .flatMap(Function.identity()) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
I think you don't need to collect all items and findAny will be enough here
There was a problem hiding this comment.
Yeah, it can be simpler a bit.
config
.getEnvironments()
.values()
.stream()
.flatMap(env -> env.getMachines().values().stream()) //convert stream of envs to stream of machines
.flatMap(machine -> machine.getInstallers())// convert to a stream of installers
.blahblah() // find if there is at least 1 entry
And also:
wsNext = attributes.containsKey(PLUGINS_ATTRIBUTE) || attributes.containsKey(EDITOR_ATTRIBUTE)
There was a problem hiding this comment.
Thanks for the installers advice, I'll make that change. For the attributes, containsKey won't work because if e.g. a user toggles plugins on and off while creating a workspace it will add the attribute "plugins" with an empty key, and this can't be fixed during workspace creation.
| * Check that workspace has either plugins defined in attributes (Che 7) or installers defined in | ||
| * machines (Che 6). | ||
| * | ||
| * @throws ValidationException |
There was a problem hiding this comment.
Please add few words when this exception can be thrown
| private static final Pattern VOLUME_NAME = Pattern.compile("[a-z][a-z0-9]{1,18}"); | ||
| private static final Pattern VOLUME_PATH = Pattern.compile("/.+"); | ||
|
|
||
| static final String PLUGINS_ATTRIBUTE = "plugins"; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah didn't see, thanks. Will fix.
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
benoitf
removed
the
status/code-review
5 participants
What does this PR do?
Checks if a workspace config contains both ws.next config options (editor, plugins) and installers during validation.
One potential shortcoming of the way this is currently implemented is that validation only occurs on creation/modification, so existing workspace with the issue will continue to launch. @garagatyi is this something we should also fix, or no?
What issues does this PR fix or reference?
#11259