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

Do not throw error when matchLabels do not find any secret/configmap #426

Closed
Shawcs opened this issue Aug 29, 2024 · 7 comments · Fixed by #432
Closed

Do not throw error when matchLabels do not find any secret/configmap #426

Shawcs opened this issue Aug 29, 2024 · 7 comments · Fixed by #432
Labels
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

@Shawcs
Copy link

Shawcs commented Aug 29, 2024

Hi team,

when we specify a selector for configMap or Secret source like this:

metadata:
  name: ca-bundle
spec:
  sources:
    - configMap:
        key: ca.crt
        selector:
          matchLabels:
            trust-bundle.certs: includes
target:
    configMap:
      key: ca-bundle.crt
    secret:
      key: ca-bundle.crt

If no configMap matches this label selector the operator throw this kind of error:

time=2024-08-29T13:40:51.076Z level=ERROR msg="failed to build source bundle" bundle=ca-bundle logger=trust/bundle err="invalid PEM data in source: bundle contains no PEM certificates"
time=2024-08-29T13:40:51.077Z level=ERROR msg="Reconciler error" controller=bundles namespace="" name=ca-bundle reconcileID=687362c5-ada0-456b-b9ff-8e1314938787 logger=trust/manager err="failed to build bundle source: invalid PEM data in source: bundle contains no PEM certificates"
time=2024-08-29T13:40:51.077Z level=DEBUG+3 msg="Failed to build bundle sources: invalid PEM data in source: bundle contains no PEM certificates" logger=trust/manager/events type=Warning object="{Kind:Bundle Namespace: Name:ca-bundle UID:fd490c64-5da0-47ea-add8-fdbb7e756daf APIVersion:trust.cert-manager.io/v1alpha1 ResourceVersion:309025552 FieldPath:}" reason=SourceBuildError
time=2024-08-29T13:40:51.077Z level=DEBUG+3 msg="Failed to build bundle sources: invalid PEM data in source: bundle contains no PEM certificates" logger=trust/manager/events type=Warning object="{Kind:Bundle Namespace: Name:ca-bundle UID:fd490c64-5da0-47ea-add8-fdbb7e756daf APIVersion:trust.cert-manager.io/v1alpha1 ResourceVersion:309025552 FieldPath:}" reason=SourceBuildError

Proposition

if we don't find any ConfigMap or Secret matching the label we just ignore it ?

@SgtCoDFish SgtCoDFish added 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. labels Sep 3, 2024
@erikgb
Copy link
Contributor

erikgb commented Sep 10, 2024

@Shawcs I have created #432 to address this issue, but I have a feeling the PR might not solve your issue. Are you expecting an empty result bundle if no sources are selected by your source selector(s)? It would be nice to know more about your overall use case!

@Shawcs
Copy link
Author

Shawcs commented Sep 10, 2024

Hi @erikgb thanks for your work on this subject.
I think there is two option here:

  • The bundle create nothing at all, if all the source selected are empty (perhaps a specific status should be rendered in the CR in this case)
  • The bundle create the desired configMap or secret in the targeted namespace with an empty key

My detail use case is this one:

A central team create a bunch of Bundle that will provide inside "clients" namespace a set of configmap/secrets for them to use in different case ( a truststore for internal calls, a truststore for specific external services, custom cert from a specific project/team). Has our service grow, new certificat will be added. Our bundle will look for confimap and secret with a specific label to add those always evolving set of certificats.
In a spirit of always configuring the same thing all bundle will be configured in the same way but only the searched labels will change, and some time we will not have configmap or secret for a given label

@erikgb
Copy link
Contributor

erikgb commented Sep 10, 2024

@Shawcs thanks for your prompt feedback! Since there is already a test asserting that an empty resulting bundle (containing no certificates) is invalid (an error), I think we should head for option one:

The bundle create nothing at all, if all the source selected are empty (perhaps a specific status should be rendered in the CR in this case)

@Shawcs
Copy link
Author

Shawcs commented Sep 10, 2024

That would do it. The only thing to be careful with is:

If I select configMap and Secret with a label and there is some configMap matching my filter but no secret I would expect the target to be created and containing the cert from the ConfigMap

@erikgb
Copy link
Contributor

erikgb commented Sep 10, 2024

If I select configMap and Secret with a label and there is some configMap matching my filter but no secret I would expect the target to be created and containing the cert from the ConfigMap

Makes a lot of sense! That is a test added to the PR - since that use case is currently broken.

@Shawcs
Copy link
Author

Shawcs commented Sep 10, 2024

I also noticed something else that can be linked to this feature. The status do not reflect problem with bad/empty source.

How to reproduce:

  1. create a bundle that target a ConfigMap with a valid cert entry
apiVersion: trust.cert-manager.io/v1alpha1
kind: Bundle
metadata:
  name: my-bundle
spec:
  sources:
    - configMap:
        key: ca.crt
        selector:
          matchLabels:
            trust-bundle.my-bundle: include
  target:
    additionalFormats:
      pkcs12:
        key: ca.p12
        password: changeit
    namespaceSelector:
      matchLabels:
        kubernetes.io/metadata.name: infra
    secret:
      key: ca.crt
kind: ConfigMap
apiVersion: v1
metadata:
  name: my-cert.crt
  namespace: infra-cert-manager
  labels:
    trust-bundle.my-bundle: include
data:
  ca.crt: |
    -----BEGIN CERTIFICATE-----
    MIIDojCCAYqgAwIBAgIQV5ocp05c1d2ULNLEDrdCpTANBgkqhkiG9w0BAQsFADBH
    MQswCQYDVQQGEwJDSDEnMCUGA1UEChMeQmFucXVlIExvbWJhcmQgT2RpZXIgZXQg
    Q2llIFNBMQ8wDQYDVQQDEwZMTyBEUEkwHhcNMjQwMzA3MDAwMDAwWhcNMjUwMzA3
    MjM1OTU5WjAVMRMwEQYDVQQDEwpnaXRodWIuY29tMFkwEwYHKoZIzj0CAQYIKoZI
    zj0DAQcDQgAEQrPuGOisrWzPTzsVzujNAMvKeM1GRDs18c2N5R6LemewOMjO0Ep1
    yESxF/xn4Zj7tlsTeMT5zz4Li1DQN/K1zKOBhjCBgzAdBgNVHQ4EFgQUO2g/NDr1
    RzTK76ZOPZq9Xm56zJ8wDgYDVR0PAQH/BAQDAgeAMAwGA1UdEwEB/wQCMAAwHQYD
    VR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMCUGA1UdEQQeMByCCmdpdGh1Yi5j
    b22CDnd3dy5naXRodWIuY29tMA0GCSqGSIb3DQEBCwUAA4ICAQBEaD1pszjmbtjc
    nE0s43FN2oU/S23Wf331M6Ae72F9B3ceqY/X0oPIHsOtpA7gSDOzjs4sNrHq34qn
    q3SRpmiDepmY4Ba2+gXNg5dWeul2e7ht22L/jYR8oT0pGClWkLiveijBT3Veqbxu
    jT3lxYGd1ey20feg4rQIw61GgaZ2dyHjlpj8FrCxSSLnULIM4db+04+2PXbTGl9J
    O+UQzbDI34KoKKWCCvCDFziCTG3rblv3RcGOCUcAXL1WpJIPlhhSYK+Dvv1Op2/C
    bd8LU10GmzZpZ/aR4SZDymggc32xvXEfPSrM36qgDcbg7Vb3mm+uZUgWKHmh1pnA
    1DeiFM9PT9GGN8m5ioMsbYIbNZUUw2jt8Gbz0CWSQlJWAfo0LEbVnIH4TG10m6Ix
    zqDgzL7QYe7XpGqY1LuwNidozLvRUaZkhDud7XlgHYYQwrP4z6/ekawett0LdvSL
    JdSRFvE7pO4K2kXxFr8YzeET0jrZ+JeVX8WYbsfUlo0UdMVnViIFrgbTd8v6Mlev
    kBG18BDEDA8hYS1JsSmpeiy/c/WsIzihgln25RcaaUPZBszu/yQp3WwtNVJojqnF
    16LisgMdnA/1gj801KmoTYUfQFTPVkITGjYVogYGZ1B9FYNwl7ymulCRrVNOzqs/
    G3VnreyXrukwRbMI/MR4ccXl5/n+Hw==
    -----END CERTIFICATE-----
  1. the bundle status should change to :
  conditions:
    - lastTransitionTime: '2024-09-10T13:24:17Z'
      message: 'Successfully synced Bundle to namespaces that match this label selector: kubernetes.io/metadata.name=infra'
      observedGeneration: 1
      reason: Synced
      status: 'True'
      type: Synced
  1. go to the source cert in the configMap and "destroy" it by adding some characters inside the cert
kind: ConfigMap
apiVersion: v1
metadata:
  name: my-cert.crt
  namespace: infra-cert-manager
  labels:
    trust-bundle.my-bundle: include
data:
  ca.crt: |
    -----BEGIN CERTIFICATE-----
    MIIDojCCAYqgAwIBAgIQV5ocp05c1d2ULNLEDrdCpTANBgkqhkiG9w0BAQsFADBH
    MQswCQYDVQQGEwJDSDEnMCUGA1UEChMeQmFucXVlIExvbWJhcmQgT2RpZXIgZXQg
    Q2llIFNBMQ8wDQYDVQQDEwZMTyBEUEkwHhcNMjQwMzA3MDAwMDAwWhcNMjUwMzA3
    MjM1OTU5WjAVMRMwEQYDVQQDEwpnaXRodWIuY29tMFkwEwYHKoZIzj0CAQYIKoZI
    PlhhSYK+Dvv1Op2/C
    bd8LU10GmzZpZ/aR4SZDymggc32xvXEfPSrM36qgDcbg7Vb3mm+uZUgWKHmh1pnA
    1DeiFM9PT9GGN8m5ioMsbYw2jt8Gbz0CWSQlJWAfo0LEbVnIH4TG10m6Ix
    zqDgzL7QYe7XpGqY1LuwNidozLvRUaZkhDud7XlgHYYQwrP4z6/ekawett0LdvSL
    JdSRFvE7pO4K2kXxFr8YzeET0jrZ+JeVX8WYbsfUlo0UdMVnViIFrgbTd8v6Mlev
    kBG18BDEDA8hYS1JsSmpeiy/c/WsIzihgtNVJojqnF
    16LisgMdnA/1gj801KmoTYUfQFNwl7ymulCRrVNOzqs/
    G3VnreyXrukwRbMI/MR4ccXl5/n+Hw==
    -----END CERTIFICATE-----
  1. the status is still the same in the bundle (and is not re generated in destination namespace), but in the operator we can read:
time=2024-09-10T13:37:16.460Z level=ERROR msg="Reconciler error" controller=bundles namespace="" name=bundle-mycert reconcileID=efb1bda3-b71d-4bed-9613-60a8b5b2b5bb logger=trust/manager err="failed to build bundle source: invalid PEM data in source: bundle contains no PEM certificates"
time=2024-09-10T13:37:16.460Z level=DEBUG+3 msg="Failed to build bundle sources: invalid PEM data in source: bundle contains no PEM certificates" logger=trust/manager/events type=Warning object="{Kind:Bundle Namespace: Name:bundle-mycert UID:debfc06e-e120-43e6-a78b-8efd646eaf7d APIVersion:trust.cert-manager.io/v1alpha1 ResourceVersion:333113243 FieldPath:}" reason=SourceBuildError
time=2024-09-10T13:37:16.460Z level=DEBUG+3 msg="Failed to build bundle sources: invalid PEM data in source: bundle contains no PEM certificates" logger=trust/manager/events type=Warning object="{Kind:Bundle Namespace: Name:bundle-mycert  UID:debfc06e-e120-43e6-a78b-8efd646eaf7d APIVersion:trust.cert-manager.io/v1alpha1 ResourceVersion:333113243 FieldPath:}" reason=SourceBuildError
time=2024-09-10T13:42:44.141Z level=ERROR msg="failed to build source bundle" bundle=bundle-mycert  logger=trust/bundle err="invalid PEM data in source: bundle contains no PEM certificates"

I suggest that when we have this kind of error the status in the bundle is updated with a relevant error. This also work for the case where source is empty

@erikgb
Copy link
Contributor

erikgb commented Sep 10, 2024

I suggest that when we have this kind of error the status in the bundle is updated with a relevant error. This also work for the case where source is empty

I couldn't agree more! 👍 Do you mind opening up a new issue asking for this? It will slightly change the behavior of the trust-manager API, and we should track such changes well. It might break some user workflows, and we should communicate such changes well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging a pull request may close this issue.

3 participants