-
Notifications
You must be signed in to change notification settings - Fork 23
validate the presence of specified issuer or clusterisssuer in tlspolicy #594
validate the presence of specified issuer or clusterisssuer in tlspolicy #594
Conversation
2382665
to
0e5eb14
Compare
0e5eb14
to
fcdf86d
Compare
fcdf86d
to
7b154db
Compare
7b154db
to
696a116
Compare
696a116
to
14d5613
Compare
14d5613
to
714bd28
Compare
the e2e tests are passing locally for me. so i'm looking into that. |
714bd28
to
884311c
Compare
884311c
to
b2aaf53
Compare
b2aaf53
to
0f193e3
Compare
0f193e3
to
9df2972
Compare
9df2972
to
cad6839
Compare
cad6839
to
e57330f
Compare
Couple of comments, but looks fine i think. It did make me wonder if this is the best way to do this kind of thing, ideally we wouldn't be doing validation that is expected to be done by cert manager (the thing this policy is wrapping), but rather rely on aggregating the status of created cert manager resources (Certificates), or expected generated resources (CertificateRequests), since this would be a truer reflection of the state of the policy. Example CertificateRequests status(cert manager 1.13.0), that we could get this info from:
Since we aren't going to do that aggregation now, and will likely be done in line with the proposed kuadrant policy status/conditions, a basic check like this is probably fine to avoid unnecessary confusion in the meantime. |
e57330f
to
b024117
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: laurafitzgerald, mikenairn The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What
Closes #581
Verification
Follow https://docs.kuadrant.io/getting-started/
Follow https://docs.kuadrant.io/multicluster-gateway-controller/docs/how-to/multicluster-gateways-walkthrough/ until the creation of the TLSPolicy
Error should be reported in status for ClusterIssuer that doesn't exist in cluster
Create TLSPolicy pointing to ClusterIssuer with a name that doesn't exist in the cluster
Validate that error is present in the status of the TLSPolicy
Error should be reported in status for Issuer that doesn't exist in the namespace of the tlspolicy
Create an issuer in a namespace that is different to the TLSPolicy - steps here to do that https://github.com/Kuadrant/multicluster-gateway-controller/blob/main/docs/tlspolicy/tls-policy.md#lets-encrypt-issuer-for-route53-hosted-domain - update the namespace in the issuer in the steps to another namespace in the cluster
Validate that error is present in the status of the TLSPolicy
Error should NOT be reported in status for Issuer that does exist in the same namespace as the tlspolicy
Create the same issuer but in the same namespace as the tlspolicy e.g. multi-cluster-gateways
Validate the status is good
Error should NOT be reported in status for ClusterIssuer that does exist in the same namespace as the tlspolicy
Update the policy to have a valid ClusterIssuer glbc
e.g.
Validate the status is good
Error should NOT be reported in status for Issuer that does not specify kind
Update the policy to have the following issuerRef - create the issuer in the same namespace if needed
e.g.
Validate the status is good