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

Add self-signed certificate init container #108

Closed
wants to merge 2 commits into from
Closed

Add self-signed certificate init container #108

wants to merge 2 commits into from

Conversation

PurseChicken
Copy link
Contributor

@PurseChicken PurseChicken commented Jun 21, 2023

Definition of Ready

  • I am happy with the code
  • Short description of the feature/issue is added in the pr description
  • PR is linked to the corresponding user story
  • Acceptance criteria are met
  • All open todos and follow ups are defined in a new ticket and justified
  • Deviations from the acceptance criteria and design are agreed with the PO and documented.
  • No debug or dead code
  • My code has no repetitions
  • Documentation/examples are up-to-date
  • All non-functional requirements are met
  • If possible, the test configuration is adjusted so acceptance tests cover my changes

This PR adds the ability to enable an init container which will generate and store a self signed certificate for each pod that runs in the replica set.

The certificate is signed with the CN of "zitadel" and contains the following SAN's: localhost, Pod IP Address and Pod Name. Additionally, it can contain one more DNS name, specified in values, which will be added to the SAN.

By default, this is disabled in the values file, but if required can be enabled by setting selfSignedCert.enabled to true. You can add the DNS name you want to add the certificate in selfSignedCert.additionalDnsName. Omit this value if this is not required.

The init container uses a small alpine/openssl image to generate the certificate using the following command:

openssl req -nodes -x509 -sha256 -newkey rsa:4096 -keyout /etc/tls/tls.key -out /etc/tls/tls.crt -days 3560 -subj "/CN=zitadel" -addext "subjectAltName = DNS:localhost,DNS:${POD_IP},DNS:${POD_NAME}"

Enabling selfSignedCert also adds the correct volume and volumeMount for the /etc/tls directory. This is where the tls.crt and tls.key files will be stored. They can then be referenced in zitadel config directly. E.G. KeyPath: /etc/tls/tls.key CertPath: /etc/tls/tls.crt

@PurseChicken PurseChicken marked this pull request as ready for review June 21, 2023 22:21
@jessebot
Copy link
Contributor

jessebot commented Aug 20, 2023

This would be great! After this branch has been rebased and possibly reviewed/merged, then we could use this feature in the values.yaml right?

zitadel:
  configmapConfig:
    TLS:
      Enabled: true
      
selfSignedCert:
  enabled: true
   additionalDnsName: zitadel.example.com

My only question is how can we rotate this cert periodically? And is there a way to use cert-manager to do this?

Also, if this gets run on every init, and you have multiple pods, wouldn't this overwrite the existing cert, or would it error if there's already a cert there?

@PurseChicken
Copy link
Contributor Author

PurseChicken commented Aug 20, 2023

This would be great! After this branch has been rebased and possibly reviewed/merged, then we could use this feature in the values.yaml right?

zitadel:
  configmapConfig:
    TLS:
      Enabled: true
      
selfSignedCert:
  enabled: true
   additionalDnsName: zitadel.example.com

My only question is how can we rotate this cert periodically? And is there a way to use cert-manager to do this?

Also, if this gets run on every init, and you have multiple pods, wouldn't this overwrite the existing cert, or would it error if there's already a cert there?

You are partly correct.

Setting selfSignedCert.enabled: true will only enable the init container to generate the cert and store it at /etc/tls/tls.crt along with the key file at /etc/tls/tls.key. If you want to use them, then you also need to set your TLS config to use them. E.G.

zitadel:
  configmapConfig:
    TLS:
      Enabled: true
      KeyPath: /etc/tls/tls.key
      CertPath: /etc/tls/tls.crt

My only question is how can we rotate this cert periodically? And is there a way to use cert-manager to do this?

The cert is valid in the pod for as long as the pod is alive. Once the pod is restarted or deleted and recreated, then the init container runs again and regenerates the certificate. That effectively rotates the certificate and key.
I am sure that this could be expanded upon to use cert-manager, but I did not include that in my PR. Since this is strictly for generating a self-signed certificate, then this solution seemed to fit the bill.

Also, if this gets run on every init, and you have multiple pods, wouldn't this overwrite the existing cert, or would it error if there's already a cert there?

The pods each use their own volume mounts and certificates. They are not shared across pods. There would be no "overwriting", just unique certificate and key for each running pod.

@PurseChicken PurseChicken closed this by deleting the head repository Sep 13, 2023
@eliobischof
Copy link
Member

@PurseChicken you closed the PR. Is it not relevant for you anymore or was there not enough response? Sorry for the delay.

@PurseChicken
Copy link
Contributor Author

I inadvertently closed this. Its still relevant and should be re-opened.

@PurseChicken
Copy link
Contributor Author

@eliobischof Can you reopen?

@eliobischof
Copy link
Member

eliobischof commented Sep 13, 2023

I can't because GitHub says that the fork was deleted (even though I can see the fork exists 🧐).
But I can create a new PR based on the changes in this PR.

@PurseChicken
Copy link
Contributor Author

Sounds good. Thanks!

@PurseChicken
Copy link
Contributor Author

Any update on this?

@PurseChicken
Copy link
Contributor Author

@eliobischof bump. Should I just create a new PR?

@eliobischof
Copy link
Member

@eliobischof bump. Should I just create a new PR?

If you have the time, I'appreciate it 🙏

@PurseChicken
Copy link
Contributor Author

This has been resubmitted in #140

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.

3 participants