Image volume mounts#4302
Conversation
| // | ||
| // +kubebuilder:validation:XValidation:rule=`has(self.claimName) != has(self.image)`,message=`you must set only one of image or claimName` | ||
| // +kubebuilder:validation:XValidation:rule=`!has(self.image) || !has(self.readOnly) || self.readOnly`,message=`readOnly cannot be set false when using an ImageVolumeSource` | ||
| // +kubebuilder:validation:XValidation:rule=`!has(self.image) || (self.?image.reference.hasValue() && self.image.reference.size() > 0)`,message=`if using an ImageVolumeSource, you must set a reference` |
There was a problem hiding this comment.
😩 So the upstream type lacks +required and instead puts Required: in the description, eh?
There was a problem hiding this comment.
And the comment describes that it's not required because it might be overwritten (by the spec it's in?), which doesn't quite explain things to me. I mean, I guess it behaves somewhat like a RELATED_ image field in OLM.
| // | ||
| // +kubebuilder:validation:XValidation:rule=`has(self.claimName) != has(self.image)`,message=`you must set only one of image or claimName` | ||
| // +kubebuilder:validation:XValidation:rule=`!has(self.image) || !has(self.readOnly) || self.readOnly`,message=`readOnly cannot be set false when using an ImageVolumeSource` | ||
| // +kubebuilder:validation:XValidation:rule=`!has(self.image) || (self.?image.reference.hasValue() && self.image.reference.size() > 0)`,message=`if using an ImageVolumeSource, you must set a reference` |
There was a problem hiding this comment.
🤔 Is there a case where the image.reference would have a value but the size would be 0?
There was a problem hiding this comment.
The original ImageVolumeSource does not have any length requirements on Reference so someone could technically put an empty string... I haven't tested it myself, but the comments in the struct say "behaves in the same way as pod.spec.containers[*].image" so I imagine it fails in some fashion if it cannot find an image.
There was a problem hiding this comment.
But would it be possible to pass an empty string through on Reference (which has omitempty)?
There was a problem hiding this comment.
The rule checks that the length of the string is greater than 0...
benjaminjb
left a comment
benjaminjb
left a comment
There was a problem hiding this comment.
LGTM, no blockers, though want to check with our tester...
| // | ||
| // +kubebuilder:validation:XValidation:rule=`has(self.claimName) != has(self.image)`,message=`you must set only one of image or claimName` | ||
| // +kubebuilder:validation:XValidation:rule=`!has(self.image) || !has(self.readOnly) || self.readOnly`,message=`readOnly cannot be set false when using an ImageVolumeSource` | ||
| // +kubebuilder:validation:XValidation:rule=`!has(self.image) || (self.?image.reference.hasValue() && self.image.reference.size() > 0)`,message=`if using an ImageVolumeSource, you must set a reference` |
There was a problem hiding this comment.
📝 I'm not finding any practical limit on the size/length of an image reference.
Add validation to ensure that user must choose between an image volume source and a pvc claim. Add validation to ensure that user cannot set readOnly to false when using an image volume source. Add validation to ensure that user sets a reference when using an image volume source. Add tests for using ImageVolumeSource in AdditionalVolume feature.
dsessler7
force-pushed
the
image-volume-mounts
branch
from
1e8ad02 to
5219488
Compare
3 participants
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
Additional volumes can only use persistent volume claims.
What is the new behavior (if this is a feature change)?
Additional volumes can now use an ImageVolumeSource.
Other Information: