[9614] Seccomp Profile added to POD and Container page of all Worklods | Pod Name Validation fixed#16048
Conversation
…ds | Pod Name Validation fixed
… to Checkbox | Added title for Capabilities
…natory texts | Fixed tooltip aria labels and select, adding values
…the Error on containers | Refactored the error based on those changes | Removed focus on tab based on feedback | Fixed test after using allContainers
richard-cox
left a comment
There was a problem hiding this comment.
Just a few minor questions and requests, otherwise looking good
|
|
||
| tabErrors() { | ||
| return { general: this.fvGetPathErrors(['image'])?.length > 0 }; | ||
| const tabErrors = { podSecurityContext: this.fvGetPathErrors(['podTemplateSpec.securityContext.seccompProfile.localhostProfile'])?.length > 0 }; |
There was a problem hiding this comment.
how come the image has been removed (not needed?) and localHostProfile added? i would have though it's both or neither?
There was a problem hiding this comment.
It moved to the allContainers() at line 433.
Before the validation of the "image" was only applying to the "current" container showing, there was only ONE validation for it, and so it was bugged.
The correct is to apply the validation of the "image" for each container separately, so here it has been removed and added to the containers" validation.
There was a problem hiding this comment.
Gotcha.
Missed this the first time around, generally we should avoid adding properties to the resource we will eventually save (that container.error object). Normally they would get persisted when the object is saved, but i think kube now sends a warning back. These are a bit different though
- There's already an error object, so the issue exists already
- The only way to produce the bug is to setup the form such that the errors exist --> edit as yaml --> save. This isn't something we would expect to happen often
Given above lets keep as is for the moment
There was a problem hiding this comment.
Thanks! Interesting to know. I just followed the same way that the Container Image was being done.
So to use the "spec" to control the UI is never a good idea. I think that happens on the computed "allContainers" which means that if we just use an UI Helper for the errors instead of using the "spec" would fix this problem.
As you said, it is already an issue now. Let's discuss, would be great to have an "issue" to track this one. But I think with the form changes, maybe the logic will disappear as well.
| class="row" | ||
| > | ||
| <div class="col span-6"> | ||
| <fieldset> |
There was a problem hiding this comment.
I think @mantis-toboggan-md is doing some work on the new form work which will be used an example for all new (and at some point existing) forms.
If we go with the fieldset + legend pattern it would be good to also do it there. If we're not sure can you bring up in the next demo session?
There was a problem hiding this comment.
That seems like a good idea to share about it and why we should use fieldset + legends.
Instead of demo session, maybe we can have a meeting between us(me, you and @mantis-toboggan-md) first and discuss about it. Than after we got agreement or direction we can share on the demo.
There was a problem hiding this comment.
Ok, we can shortcut and start a slack discussion in our team channel? I think it should be easy to get consensus there. Should be straight forward if Alex and myself are onboard
…dded comment for fvFormRulesets | created FORM_TYPES for the Security component
|
Changed applied as requested: Merged master to fix conflict |

Summary
Fixes #9614
Occurred changes and/or fixed issues
#9614
Technical notes summary
Areas or cases that should be tested
Issue 9614 - 2.pdf
Areas which could experience regressions
Screenshot/Video
Issue 9614 - 2.pdf
Issue 9614 - 2 - UI Check.pdf
Checklist
Admin,Standard UserandUser Base