-
Notifications
You must be signed in to change notification settings - Fork 614
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
[RFC] Attributes in image automation #2222
base: main
Are you sure you want to change the base?
Conversation
It would be nice if image SHA256 available. |
@GregoireW I'm sorry for the delay on this, I've started to review this and I'd like to resume the discussions. If you're still interested in contributing, would you be able to reach out to me in CNCF Slack workspace? |
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.
Here are some suggestions after a first pass, I hope these help.
Thank you for putting this together!
|
||
## Summary | ||
|
||
Image automation controller can update some attributes of a kubernetes object. |
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.
Would you mind changing this to something that more directly explains what the RFC covers? Something like "Flux should allow referencing more container image metadata attributes in the image automation Setters strategy"
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.
I rewrote a little bit this part.
|
||
### Goals | ||
|
||
This RFC aims to describe a way to extract such additional value from the |
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.
Maybe change this to a list and add the digest information extraction under the umbrella of this RFC:
- Allow referencing
filterTags
pattern extracted parts from image policies - Extract digest information and make it available in the markers Add image digests image-reflector-controller#214
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.
I rewrote a little bit this part. But the Additional digest requires much more work. It would means downloading the image manifest for every tag then filter the correct one.
Manifest wouls also means access to OCI label and cleaner filter / attributes, but it may increase the database size (some build tools put a lot of data in label!)
|
||
```yaml | ||
extract: $ts | ||
pattern: ^pr-(?P<pr>.*)-(?P<ts>\d*)-(?P<sha1>.*)$ |
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.
What would happen if we'd just use regular capture groups instead of named groups? Maybe we should be able to support that as well by referencing the indices?
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.
I'm not found of regular capture group when the use of the capture is not in the same code space as the capture itself. The definition is on the ImagePolicy then the use is on a comment of another kubernetes object. What's 1, 2, 3 in this context... not sure. (I Am Not A Number, I Am A Free Man! )
It is possible to modify the image automation to take comment like: | ||
|
||
```yaml | ||
# {"$imagepolicy": "{namespace}:{imagepolicy}:{attributes}" |
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.
It might make sense to delimitate the attributes extracted from .spec.filterTags.pattern
from the others to avoid any collisions or confusion.
I'm proposing we go with: # {"$imagepolicy": "{namespace}:{imagepolicy}:tagparts[{attribute}]"
, where attribute
can be either a named capture group or a index number.
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.
I think there is 2 aspects to consider:
The simplicity would means to use {namespace}:{policy}:{attribute}
simple to read and to understand. I added a part on collision where a warning should be logged, but the value should remains as today norm.
The tag parts would be interesting if in the future multiple way to retrieve data would be permitted. For instance label from OCI container ... if you then be interesting to split "extracted attributes" to "manifest attributes". Or perhaps it would simply be a configuration on the ImagePolicy like "attributes X = json path in the manifest" which would kept Image Automation Controller simple.
|
||
with `attributes` a name of a capture group on the pattern. | ||
|
||
From previous pattern example, accepted attributes will be: |
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.
What should happen if the user would reference a non-existent attribute?
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.
I guess a warning in the log and no update. BTW a capture producing empty attributes will need to be tested
but heavier to build. | ||
This raise the question on should this be included in flux or not. | ||
|
||
## Design Details |
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.
Since this heavily depends on what information image policies expose I think we need to detail how this would be implemented in the IRC as well.
You mention you are interrested by the image digest. Can you elaborate on your use case ? |
Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>
Signed-off-by: gregoireW <24318548+GregoireW@users.noreply.github.com>
Co-authored-by: Aurel Canciu <aurelcanciu@gmail.com>
Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>
Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>
Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>
Is flux image automation a good solution to extract additional data from image tag?
Signed-off-by: GregoireW 24318548+GregoireW@users.noreply.github.com