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

fix: make notation verifier installation optional on ratify installation #1719

Merged
merged 4 commits into from
Aug 29, 2024
Merged

fix: make notation verifier installation optional on ratify installation #1719

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f12b6bb
feat: make notation verifier installation optional on ratify installa…
shahramk64 Aug 19, 2024
d8febbb
feat: show a warning message when no verifier is enabled for installa…
shahramk64 Aug 28, 2024
5447f92
fix: remove the redundant 'NOTES' word
shahramk64 Aug 28, 2024
c33763d
Merge branch 'dev' into make-notation-verifier-installation-optional
binbin-li Aug 29, 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
7 changes: 7 additions & 0 deletions charts/ratify/templates/NOTES.txt
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@susanshi Should we only check for cosign and notation here? Or sbom should also be included?

Copy link
Collaborator

Choose a reason for hiding this comment

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

all external plugin should be included too

Copy link
Collaborator

Choose a reason for hiding this comment

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

curious: does this block print out a warning on helm install?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Helm doesn't provide a straightforward way to issue a warning that doesn't halt execution, initially I was just printing a string based on the same condition on top of the verifier.yaml file. But it doesn't work, because Helm treated the printed warning message as part of the Kubernetes resource definition.
WE can leverage Helm's NOTES.txt file, which is specifically designed for displaying messages after a successful installation or rendering. While this doesn't show up directly during the template rendering phase, it will ensure the user sees the message right after installation.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{{- if not (or .Values.notation.enabled .Values.cosign.enabled .Values.sbom.enabled) }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can add check on vulnerabilityreport as well

NOTES:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: NOTES can be removed as this would be auto-generated by helm. Otherwise it will look like:
image

***********************************************************
WARNING: All verifiers are disabled.
It's recommended that at least one is enabled for proper functionality.
***********************************************************
{{- end }}
2 changes: 2 additions & 0 deletions charts/ratify/templates/verifier.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{{- $fullname := include "ratify.fullname" . -}}
{{- if .Values.notation.enabled }}
apiVersion: config.ratify.deislabs.io/v1beta1
kind: Verifier
metadata:
Expand Down Expand Up @@ -37,6 +38,7 @@ spec:
- ca:certs
trustedIdentities:
- "*"
{{- end }}
---
{{- if .Values.cosign.enabled }}
apiVersion: config.ratify.deislabs.io/v1beta1
Expand Down
3 changes: 3 additions & 0 deletions charts/ratify/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ tolerations: []
notationCerts: []
cosignKeys: []

notation:
enabled: true

cosign:
enabled: true
scopes: ["*"] # corresponds to a single trust policy
Expand Down
Loading