Skip to content

fix: grafana ingress class - #85

Merged
Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
grafana-ingress-class
Apr 17, 2024
Merged

fix: grafana ingress class#85
Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
grafana-ingress-class

Conversation

@kvaps

Copy link
Copy Markdown
Member

Grafana uses default ingress class nginx specified in ClusterIssuer resource, we need to overwrite it to correct ingress class from namespace annotation

metadata:
annotations:
kubernetes.io/ingress.class: "{{ $ingress }}"
acme.cert-manager.io/http01-ingress-class: "{{ $ingress }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was sure that the ingress class would be taken from ingressClassName field. Are you sure that it is necessary?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I also tried to omit ingressClass from cluster issuer, unfortunately it didn't work.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ingressClass from cluster issuer, unfortunately it didn't work.

it should be only in Cluster Issuer, right?

@kvaps Andrei Kvapil (kvaps) Apr 17, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Dunno, when cluster issure missing the section

    solvers:
    - http01:
        ingress:
          class: nginx

cert-manager reports an error:

Failed to determine a valid solver configuration for the set of domains on the Order: no configured challenge solvers can be used for this challenge

even with this annotation

@@ -68,6 +68,7 @@ spec:
metadata:
annotations:
kubernetes.io/ingress.class: "{{ $ingress }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you could bravely remove it - it is obsolete

@kvaps Andrei Kvapil (kvaps) Apr 17, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sure, done

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
@kvaps
Andrei Kvapil (kvaps) merged commit 2d21ed6 into main Apr 17, 2024
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.

2 participants