Skip to content
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.

Sign up for GitHub

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

[RFC] Attributes in image automation #2222

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bf0c470
RFC0005 - attributes in image automation
GregoireW Dec 16, 2021
ea4a1a4
Implementation hint
GregoireW Dec 17, 2021
6d5f02a
Update rfcs/0005-managed-attributes-image-automation/README.md
GregoireW Jan 25, 2022
200b4c3
More detail for the RFC
GregoireW Jan 25, 2022
15ea533
More detail for the RFC
GregoireW Jan 25, 2022
72c9bb8
More detail for the RFC
GregoireW Jan 25, 2022
c9361ca
Merge branch 'main' into rfc0005
GregoireW Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
More detail for the RFC 
Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>
  • Loading branch information
@GregoireW
GregoireW committed Jan 25, 2022
commit 15ea533e421c36beccef3b88c9f73c6f27a20d4d
21 changes: 16 additions & 5 deletions rfcs/0005-managed-attributes-image-automation/README.md
View file
Original file line number Diff line number Diff line change
@@ -55,15 +55,23 @@ This raise the question on should this feature to be included in flux or not.

## Design Details
Copy link
Member

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.


Simple update on the image automation controller should be enough. Today a
filter in the image policy is like:
Two options are possible here:

- Only modify the Image Automation Controller to make it read ImagePolicies spec
and compute attributes
- Modify the Image Reflector Controller, to extract the attributes, stores them
in the status and update the Image Automation Controller to use this new data storage.

The second option seems to be preferable to separate concerns.

A simple option would be to allow multiple capture group in the filter in the ImagePolicy:

```yaml
extract: $ts
pattern: ^pr-(?P<pr>.*)-(?P<ts>\d*)-(?P<sha1>.*)$
Copy link
Member

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?

Copy link
Author

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:
And then to modify the Image Automation Controller to take comment like:

```yaml
# {"$imagepolicy": "{namespace}:{imagepolicy}:{attributes}"
Copy link
Member

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.

Copy link
Author

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.

@@ -77,8 +85,11 @@ From previous pattern example, accepted attributes will be:
- ts
- sha1

If a user try to use an attribute name like `tag` or `name` which is
already defined by flux core, then the original meaning will still be kept :
If a user try to capture an attribute with a name like `tag` or `name` (already defined
by flux core), then the original value will be kept and a warning should show on the
Image Reflector Controller logs.

As reminder, here is the definition for those default attributes:

- tag: the full tag string
- name: the image name