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-manager-app fixes #381

Merged
merged 35 commits into from
Nov 2, 2023
Merged

Cert-manager-app fixes #381

merged 35 commits into from
Nov 2, 2023

Conversation

ssyno
Copy link
Contributor

@ssyno ssyno commented Oct 25, 2023

This PR:
resolves #28598

  • fixes issues around cert-manager-app
    • IngressClass nginx has to exist for the acme-solvers to work
    • ClusterIssuers exist after job completes
    • NetworkPolicies & CiliumNetworkPolicies for giantswarm namespace

Checklist

  • Update changelog in CHANGELOG.md.

@ssyno ssyno requested a review from a team October 25, 2023 17:42
@ssyno ssyno changed the title Ingress class fix Cert-manager-app fixes Oct 25, 2023
@@ -46,4 +46,4 @@ acme:
secretAccessKey: ""
http01:
enabled: true
ingressClassName: nginx
ingressClass: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

For my intertest, what will be the outcome of removing the default?
Will it just result the solver ingresses to have no class or will they have the class of the original ingress? Could there be a negative outcome for cases in which other issuers/classes are defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I came to the conclusion that this change to "" or default doesn't makes sense for now, as there may be no ingress controllers deployed by the time cert-manager is deployed

helm/cert-manager/values.yaml Outdated Show resolved Hide resolved
acme.cert-manager.io/http01-solver: "true"
policyTypes:
- Ingress
egress:

This comment was marked as outdated.

@T-Kukawka
Copy link

can we merge and release this? 🙏

@ssyno ssyno merged commit a05da86 into main Nov 2, 2023
@ssyno ssyno deleted the ingress-class-fix branch November 2, 2023 12:56
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.

6 participants