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

jks keystore in secret is removed when a secret is referenced by multiple certificates #7063

Open
TrilokGeer opened this issue Jun 3, 2024 · 6 comments · May be fixed by #7064
Open

jks keystore in secret is removed when a secret is referenced by multiple certificates #7063

TrilokGeer opened this issue Jun 3, 2024 · 6 comments · May be fixed by #7064
Labels
area/api Indicates a PR directly modifies the 'pkg/apis' directory kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@TrilokGeer
Copy link
Contributor

Describe the bug:

A secret name is used in multiple certificates which causes the conflict. The policies address the conflict and update the certificate status accordingly to

    Reason:                IncorrectCertificate
    Status:                False
    Type:                  Ready

However, the secret manager tends to update the secret data when the secret is not owned by the certificate. This results in missing data in secret as per certificate spec. In this case, when there is an existing valid secret generated with jks, secret manager tends to remove and re-generate jks when there is non-jks certificate referring to the same secret is created during the reconciliation.

Expected behaviour:

If the secret annotation does not have the desired certificate name, the secret must not be updated as per the certificate spec.

Steps to reproduce the bug:

  1. Create a jks password secret.
  2. Create a certificate to generate secret with jks. Use the valid jks password created.
  3. Validate the secret data and certificate status.
  4. Create a certificate that is expected to generate only tls data with no jks data.
  5. Validate the certificate status.
  6. Check the secret keystore.jks and truststore.jks.

Anything else we need to know?:

Environment details::

  • Kubernetes version:
  • Cloud-provider/provisioner:
  • cert-manager version:
  • Install method: e.g. helm/static manifests

/kind bug

@cert-manager-prow cert-manager-prow bot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 3, 2024
@hawksight
Copy link
Member

hawksight commented Jun 6, 2024

@TrilokGeer - so in essence, you have 2 Certificate resources pointing to the same secret name?
One cert that includes JKS keystore, the second cert does not include that in the spec?

What version of cert-manager are you seeing this issue on?
In general in the latest cert-manager versions (1.13, 1.14. & 1.15) cert-manager should indicate that the secret is managed by another certificate resource. But your issue indicates this is not the case?

@hawksight
Copy link
Member

@TrilokGeer is the issue or the same as the issue I described in #6992 ?

@TrilokGeer
Copy link
Contributor Author

TrilokGeer commented Jun 20, 2024

you have 2 Certificate resources pointing to the same secret name?
One cert that includes JKS keystore, the second cert does not include that in the spec?

Yes

What version of cert-manager are you seeing this issue on?

1.13, 1.14, 1.15

cert-manager should indicate that the secret is managed by another certificate resource.

The certificate has the correct status set while processing by trigger controller. The secret update is not conditioned to check for the status in the issuing controller. Hence, the post-issuance policy chain is updated to ensure the secret is updated with the data generated for the corresponding certificate.

@maelvls
Copy link
Member

maelvls commented Jun 20, 2024

Hey, thanks for the super detailed ticket. Is that the issue reported in https://issues.redhat.com/browse/CM-339?

@TrilokGeer
Copy link
Contributor Author

Hey, thanks for the super detailed ticket. Is that the issue reported in https://issues.redhat.com/browse/CM-339?

Yes, that is correct.

@erikgb
Copy link
Contributor

erikgb commented Jul 16, 2024

/priority important-longterm
/area api

@cert-manager-prow cert-manager-prow bot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. area/api Indicates a PR directly modifies the 'pkg/apis' directory labels Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Indicates a PR directly modifies the 'pkg/apis' directory kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
4 participants