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.

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 6 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
99 changes: 99 additions & 0 deletions rfcs/0005-managed-attributes-image-automation/README.md
@@ -0,0 +1,99 @@
# RFC-0005 Extend supported list of image automation marker reference attributes

**Status:** provisional

**Creation date:** 2021-12-16

**Last update:** 2022-01-25

## Summary

Flux should allow referencing more metadata in the image automation Setters strategy.

## Motivation

Some automation or observability tools can use label to identify better a
kubernetes object. It can be linked to a version, a date, a code
origin... For multiple reason, the image tag can reflect poorly this
data. An example can be given by the image reflector controller which
can extract a part of the tag and use it to sort and select the correct one.

### Goals

This RFC aims to describe

- A way to extract such additional attributes from the image tag.
- Use those new attributes to update the kubernetes object.

### Non-Goals

This RFC will focus on Image Automation Controller and Image Reflector Controller.

It is a non goal to keep in sync the attributes if the kubernetes object is
updated manually.

## Proposal

### User Stories

As a user, I can update the filter pattern on the image policy object to
capture additional data.
Then, I can reference the name of the captured group in the comment of a
kubernetes object so that the attribute linked to this comment can be updated.


### Alternatives

An alternative would be to build a mutation web hook which would be able to
filter all object and interact with them directly.

It would be more generic, more customizable and safer (fix the manual update use case)
to create such mutation web hook, but will be more complex to build.
(new kubernetes object, new controller)

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.


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! )

```

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.

```

with `attributes` a name of a capture group on the pattern.

From previous pattern example, accepted attributes will be:
Copy link
Member

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?

Copy link
Author

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


- pr
- ts
- sha1

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

## Implementation History

_not implemented yet_