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

Cert fix #2775

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Cert fix #2775

merged 1 commit into from
Jul 31, 2023

Conversation

tmjd
Copy link
Member

@tmjd tmjd commented Jul 28, 2023

Description

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

- Only require key usage for certain secret certs
- Compliance: order dependencies so certificates are updated before pods
  are updated.
@@ -85,6 +87,14 @@ var (
ComplianceReporterSourceEntityRule = networkpolicy.CreateSourceEntityRule(ComplianceNamespace, ComplianceReporterName)
)

// Register secret/certs that need Server and Client Key usage
func init() {
certkeyusage.SetCertKeyUsage(ComplianceServerCertSecret, []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth})
Copy link
Member

Choose a reason for hiding this comment

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

When considering the init function, what made you decide to do it in the render file, rather than the controller?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mainly because that is where the secret name is defined.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can justify it as the render is where we define the state/configuration of components, so it makes sense that we define the state/configuration of the secrets. But I'm sure we could come up with rationale that it should be in the controller.

Copy link
Member

@rene-dekker rene-dekker left a comment

Choose a reason for hiding this comment

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

lgtm

@tmjd tmjd merged commit 9eb3ddf into tigera:master Jul 31, 2023
3 checks passed
@tmjd tmjd deleted the cert-fix branch July 31, 2023 18:14
tmjd added a commit that referenced this pull request Jul 31, 2023
[pick #2775] Recreate certificates that are only specified to be used as server certs [r1.31]
tmjd added a commit that referenced this pull request Jul 31, 2023
[Pick #2775] Recreate certificates that are only specified to be used as server certs [r1.30]
@danudey danudey modified the milestones: v1.31.0, v1.32.0 Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants