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

Jump to bottom

Refine explanation of meeting regenerate after expiry requirement #28502

Merged
merged 1 commit into from
Jan 5, 2024
Merged

Refine explanation of meeting regenerate after expiry requirement #28502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 10 additions & 6 deletions ...date-tls-artifacts/generate-owners/tlsmetadata/autoregenerate_after_expiry/requirement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,22 @@ func NewAutoRegenerateAfterOfflineExpiryRequirement() tlsmetadatainterfaces.Requ
md.Text("that if the cluster is shut down until the certificate expires, when the machines are restarted")
md.Text("the cluster will automatically create new cert/key pairs or update CA bundles as required without human")
md.Text("intervention.")
md.Textf("To assert that a particular cert/key pair or CA bundle can do this, add the %q annotation to the secret or configmap and ",
annotationName)
md.Text("setting the value of the annotation a github link to the PR adding the annotation.")
md.Text("This assertion also means that you have")
md.Text("")
md.Text("To assert that a particular cert/key pair or CA bundle can do this, add the annotation to the secret or configmap.")
md.Text("```yaml")
md.Text(" annotations:")
md.Textf(" %v: https//github.com/link/to/pr/adding/annotation, \"quote escaped formatted name of e2e test that ensures the PKI artifact functions properly\"", annotationName)
md.Text("```")
md.Text("")
md.Text("This assertion means that you have")
md.OrderedListStart()
md.NewOrderedListItem()
md.Text("Manually tested that this works or seen someone else manually test that this works. AND")
md.NewOrderedListItem()
md.Text("Written an automated e2e job that your team has an alert for and is a blocking GA criteria, and/or")
md.Text("Written an automated e2e test to ensure this PKI artifact is function that is a blocking GA criteria, and/or")
md.Text("QE has required test every release that ensures the functionality works every release.")
md.OrderedListEnd()
md.Text("Links should be provided in the PR adding the annotation.")
md.Text("If you have not done this, you should not merge the annotation.")

return tlsmetadatainterfaces.NewAnnotationRequirement(
// requirement name
Expand Down
5 changes: 4 additions & 1 deletion pkg/cmd/update-tls-artifacts/generate-owners/tlsmetadata/descriptions/requirement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ func NewDescriptionRequirement() tlsmetadatainterfaces.Requirement {
md.Text("Which names and IPs a serving certificate terminates.")
md.NewOrderedListItem()
md.Text("Which subject (user and group) a client certificate is created for.")
md.NewOrderedListItem()
md.Text("Which binary and flags is this certificate wired to.")
md.OrderedListEnd()
md.Textf("To create a description, set the %q annotation to the markdown formatted string describing your TLS artifact. ",
md.Text("")
md.Textf("To create a description, set the `%v` annotation to the markdown formatted string describing your TLS artifact. ",
annotations.OpenShiftDescription)

return tlsmetadatainterfaces.NewAnnotationRequirement(
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/update-tls-artifacts/generate-owners/tlsmetadatainterfaces/annotation_requirement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func (o annotationRequirement) generateInspectionMarkdown(pkiInfo *certgraphapi.
}

md := NewMarkdown(o.title)
md.Title(2, "How to meet the requirement")
md.ExactText(o.explanationMD)

if len(violatingCertsByOwner) > 0 || len(violatingCABundlesByOwner) > 0 {
Expand Down
17 changes: 12 additions & 5 deletions tls/autoregenerate-after-expiry/autoregenerate-after-expiry.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Auto Regenerate After Offline Expiry

## Table of Contents
- [How to meet the requirement](#How-to-meet-the-requirement)
- [Items Do NOT Meet the Requirement (227)](#Items-Do-NOT-Meet-the-Requirement-227)
- [ (20)](#-20)
- [Certificates (9)](#Certificates-9)
Expand Down Expand Up @@ -39,17 +40,23 @@
- [Items That DO Meet the Requirement (0)](#Items-That-DO-Meet-the-Requirement-0)


## How to meet the requirement
Acknowledging that a cert/key pair or CA bundle can auto-regenerate after it expires offline means
that if the cluster is shut down until the certificate expires, when the machines are restarted
the cluster will automatically create new cert/key pairs or update CA bundles as required without human
intervention.
To assert that a particular cert/key pair or CA bundle can do this, add the "certificates.openshift.io/auto-regenerate-after-offline-expiry" annotation to the secret or configmap and
setting the value of the annotation a github link to the PR adding the annotation.
This assertion also means that you have

To assert that a particular cert/key pair or CA bundle can do this, add the annotation to the secret or configmap.
```yaml
annotations:
certificates.openshift.io/auto-regenerate-after-offline-expiry: https//github.com/link/to/pr/adding/annotation, "quote escaped formatted name of e2e test that ensures the PKI artifact functions properly"
Copy link
Member

Choose a reason for hiding this comment

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

I think just the test name should be sufficient, we can find out which PR added it with git blame (also across releases)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think just the test name should be sufficient, we can find out which PR added it with git blame (also across releases)

Based on past experience in kube with api approvals, there's nothing quite like the link embedded.

```

This assertion means that you have
1. Manually tested that this works or seen someone else manually test that this works. AND
2. Written an automated e2e job that your team has an alert for and is a blocking GA criteria, and/or
2. Written an automated e2e test to ensure this PKI artifact is function that is a blocking GA criteria, and/or
QE has required test every release that ensures the functionality works every release.
Links should be provided in the PR adding the annotation.
If you have not done this, you should not merge the annotation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If you have not done this, you should not merge the annotation.
If you have not done this, you should not merge the PR setting this annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vrutkovs based on William's comment, I'm going to merge this to help us communicate where we're at with a perma-link.

I'm completely fine adding this extra verbiage, but I need to merge with green verifies.


## Items Do NOT Meet the Requirement (227)
### (20)
Expand Down
6 changes: 5 additions & 1 deletion tls/descriptions/descriptions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Description of TLS Artifacts

## Table of Contents
- [How to meet the requirement](#How-to-meet-the-requirement)
- [Items Do NOT Meet the Requirement (139)](#Items-Do-NOT-Meet-the-Requirement-139)
- [ (20)](#-20)
- [Certificates (9)](#Certificates-9)
Expand Down Expand Up @@ -39,13 +40,16 @@
- [Certificate Authority Bundles (3)](#Certificate-Authority-Bundles-3)


## How to meet the requirement
TLS artifacts must have user-facing descriptions on their in-cluster resources.
These descriptions must be in the style of API documentation and must include
1. Which connections a CA bundle can be used to verify.
2. What kind of certificates a signer will sign for.
3. Which names and IPs a serving certificate terminates.
4. Which subject (user and group) a client certificate is created for.
To create a description, set the "openshift.io/description" annotation to the markdown formatted string describing your TLS artifact.
5. Which binary and flags is this certificate wired to.

To create a description, set the `openshift.io/description` annotation to the markdown formatted string describing your TLS artifact.

## Items Do NOT Meet the Requirement (139)
### (20)
Expand Down