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

feat(vector): Add digest option to vector image #219

Merged
merged 2 commits into from
Jun 14, 2022
Merged

feat(vector): Add digest option to vector image #219

merged 2 commits into from
Jun 14, 2022

Conversation

tomer-epstein
Copy link
Contributor

@tomer-epstein tomer-epstein commented Jun 13, 2022

Signed-off-by: Tomer Epstein [email protected]

Motivation
add better integrity to the image version, we should provide a way to add a digest option to the chart images this way we can validate the image version

Approach
How does this change address the problem?

Add digest option to vector chart,
By adding a new sha field to the pod file and concatenate it to the image string address when provided.

Pull Request status

  • Initial implementation
  • Refactoring
  • Unit tests
  • Integration tests

@spencergilbert
Copy link
Contributor

Thanks for the contribution - Digests would be usable today by just passing image.tag: 0.22.1-distroless-libc@sha256:ba208391be6f7705f5e7bf7513ad7600cf8cc2c5dff3a4691632de3afbf0bcd0, right? This is just a more friendly way of exposing it?

@tomer-epstein
Copy link
Contributor Author

tomer-epstein commented Jun 13, 2022

It's more then just a friendly way.

by passing a tag that conains a digest, the labels section in the vector chart will fail.
https://github.tools.sap/api-gateway/vector-chart-wrapper/blob/65a14f1ad36255eb3146f8df826b6221cd617d29/vector/charts/vector/templates/_helpers.tpl#L40

label values in k&s are allowing only specific syntax.
https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set

Example of using digest (and tag as comment before the digest) OR tag in grafana:
https://github.com/grafana/helm-charts/blob/main/charts/grafana/templates/_pod.tpl#L397-L401

@spencergilbert
Copy link
Contributor

It's more then just a friendly way.

by passing a tag that conains a digest, the labels section in the vector chart will fail. https://github.tools.sap/api-gateway/vector-chart-wrapper/blob/65a14f1ad36255eb3146f8df826b6221cd617d29/vector/charts/vector/templates/_helpers.tpl#L40

label values in k&s are allowing only specific syntax. https://github.tools.sap/api-gateway/vector-chart-wrapper/blob/65a14f1ad36255eb3146f8df826b6221cd617d29/vector/charts/vector/templates/_helpers.tpl#L40

Example of using digest (and tag as comment before the digest) OR tag in grafana: https://github.com/grafana/helm-charts/blob/main/charts/grafana/templates/_pod.tpl#L397-L401

Ah good point - I didn't think about that. I haven't written up a contributing doc yet, but we'll need to bump the minor version and run helm-docs to update the readme. I'm happy to do that if you'd prefer.

@tomer-epstein
Copy link
Contributor Author

Sure, go for it.
Will you do it on this PR?

@spencergilbert spencergilbert enabled auto-merge (squash) June 14, 2022 13:41
@tomer-epstein
Copy link
Contributor Author

@spencergilbert can you approve?

Copy link
Contributor

@spencergilbert spencergilbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

@spencergilbert spencergilbert merged commit cf0f4c1 into vectordotdev:develop Jun 14, 2022
@tomer-epstein
Copy link
Contributor Author

🤦
Tnx @spencergilbert
Appriciate the very quick response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants