Skip to content

Add a way to byo CA/Issuer#435

Merged
leochr merged 1 commit intomainfrom
byo-ca-and-issuer
Nov 11, 2022
Merged

Add a way to byo CA/Issuer#435
leochr merged 1 commit intomainfrom
byo-ca-and-issuer

Conversation

@arturdzm
Copy link
Copy Markdown
Contributor

@arturdzm arturdzm commented Nov 7, 2022

No description provided.

Copy link
Copy Markdown
Member

@leochr leochr left a comment

Choose a reason for hiding this comment

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

@arturdzm Looks good. Added a question about the custom issuer. Thanks.

Comment thread utils/reconciler.go
@@ -448,7 +469,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix s
bao := ba.(metav1.Object)

err = r.GenerateCMIssuer(bao.GetNamespace(), prefix, CACommonName, operatorName)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be skipped if a custom issuer exists?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I decided to leave this on just so that it is always there, and since default issuer / CA are not owned by application they will not be deleted with apps.
We might want to revisit in the future, just didn't want to add complexity of cleanup at this point

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay, let's revisit. Agree that we don't need to clean up the default issuer and its resources, but the method call GenerateCMIssuer can be skipped if a custom issuer is used.

@leochr leochr merged commit 8dd9234 into main Nov 11, 2022
@leochr leochr deleted the byo-ca-and-issuer branch April 8, 2026 19:09
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