added cert manager resources #423
Conversation
| @@ -0,0 +1,320 @@ | |||
| # Source: cert-manager/templates/crd-cert-manager.io_certificaterequests.yaml | |||
There was a problem hiding this comment.
Is there a good reason why these are not store in the crds/ folder instead of the apps/ folder?
There was a problem hiding this comment.
I must say that I find it odd to distribute things belonging together out into separate folders. It will make it harder to remove and have an overview of a single component, because the knowledge about it is spread out widely on the folder structure.
There was a problem hiding this comment.
I was inspired by this https://github.com/dfds/platform-apps/tree/main/apps/external-snapshotter-crd
so I followed the model. But ofcorse I would follow same structure.
| @@ -0,0 +1,28 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
- Can you please explain this file? I think it is the first time I have seen a bash script I have not understood at all.
- Why does it rely on a Docker container with yq, instead of yq installed?
There was a problem hiding this comment.
You are introducing processes that are manual and not automated. Now team members have to be aware of this new thing.
There was a problem hiding this comment.
the script is just to help on creating separate files because when you download the crds from the release page it will download them as a single file.
| install: | ||
| remediation: | ||
| retries: 3 | ||
| values: |
There was a problem hiding this comment.
You need to set resources requests and limits here.
There was a problem hiding this comment.
Which you ignored or removed from the templates and documentation I provided to you.
| @@ -0,0 +1,8 @@ | |||
| apiVersion: source.toolkit.fluxcd.io/v1 | |||
There was a problem hiding this comment.
I never understood the need for sources to not be with helmrelease?
| In our setup we use the the <https://github.com/dfds/infrastructure-modules/tree/master/_sub/compute/k8s-traefik-flux> Terraform module to generate the kustomization.yaml files. | ||
| ### Updating and commiting CRDs |
There was a problem hiding this comment.
Nice attempt. But this will surely be forgotten. The solution should be automated and use the version used in the helmrelease.
|
I really like having the CRDs present in the repo, so that we can see what changes are being made to them. |
| install: | ||
| remediation: | ||
| retries: 3 | ||
| values: |
|
Should we close this one, since it is not ready for review @samidbb ? |
This pull request adds support for installing and managing cert-manager alongside its Custom Resource Definitions (CRDs). It also adds instructions and script for updating cert-manager' CRDs