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

docs: add doc KMP Refresh #103

Merged
merged 1 commit into from
Sep 3, 2024
Merged

docs: add doc KMP Refresh #103

merged 1 commit into from
Sep 3, 2024

Conversation

duffney
Copy link
Contributor

@duffney duffney commented Aug 15, 2024

No description provided.

Copy link

netlify bot commented Aug 15, 2024

Deploy Preview for ratify-dev ready!

Name Link
🔨 Latest commit c5e5760
🔍 Latest deploy log https://app.netlify.com/sites/ratify-dev/deploys/66d0ce04502ce40008d456c5
😎 Deploy Preview https://deploy-preview-103--ratify-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@duffney
Copy link
Contributor Author

duffney commented Aug 15, 2024

Are there any other docs I should mention the refreshInterval in? @yizha1 @FeynmanZhou

susanshi
susanshi previously approved these changes Aug 20, 2024
Copy link
Collaborator

@susanshi susanshi left a comment

Choose a reason for hiding this comment

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

LGTM. thanks!

@@ -102,6 +102,7 @@ metadata:
name: # a unique name
spec:
type: azurekeyvault
refreshInterval: # OPTIONAL: [string], time duration to refresh the certificates/keys. Disabled by default. Example: 1h, 30m, 1h30m. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h"
Copy link
Collaborator

@susanshi susanshi Aug 21, 2024

Choose a reason for hiding this comment

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

suggestion to add an example for refresh enabled KMPs in the example section, or link to the /sample

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 I added samples to the inline and akv sections to point to the samples in the ratify repo. :) Thanks for the suggestion.

@@ -130,7 +131,7 @@ spec:

- If a key/certificate is in disabled state, KMP resource creation will FAIL. Users must remove reference to a disabled Key/Certificate or re-enable in Azure Key Vault.

- Ratify does NOT yet support periodic refresh and polling of certificates/keys. If the default latest version changes, object is disabled/expired, or deleted, Ratify will only become aware once the KMP resource is reconciled (edited, deleted, added).
- Ratify supports periodic refresh and polling of certificates/keys from Azure Key Vault. The `refreshInterval` field can be set to a time duration to refresh the certificates/keys. When no version of the certificate or key is specified, the latest version will be fetched and the resource will be updated. However, if a version is specified, the resource will be locked to that version and will not be updated.
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might also want to talk about limitation that auto fetching latest might result in verification failure if previous image signed are with other version of the cert. We can also link N version issue so reader is aware of the road map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call out, I'll add that limitation to the doc. :)

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 do you have a link to that issue? Or does one need to be created since the n-verion feature isn't included in the refresher for the 1.3 release?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't find the n-version issue, @duffney could you help create one?

Copy link
Contributor Author

@duffney duffney Aug 26, 2024

Choose a reason for hiding this comment

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

Certainly, here's a link. ratify-project/ratify#1751 :) It's also been added to the doc.

Copy link
Collaborator

@susanshi susanshi left a comment

Choose a reason for hiding this comment

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

Left some comments. thanks!

@duffney duffney changed the title Document refreshInterval for kmp Doc: KMP Refresh Aug 23, 2024
@akashsinghal akashsinghal changed the title Doc: KMP Refresh docs: KMP Refresh Aug 26, 2024
Copy link
Collaborator

@akashsinghal akashsinghal left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding it

Copy link
Collaborator

@binbin-li binbin-li left a comment

Choose a reason for hiding this comment

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

lgtm

@susanshi susanshi changed the title docs: KMP Refresh docs: add doc KMP Refresh Sep 3, 2024
@susanshi susanshi merged commit 4c2c1f9 into ratify-project:main Sep 3, 2024
4 of 5 checks passed
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.

4 participants