From e2f13f5f9cbc96c432742d5b7b94180790e09dfa Mon Sep 17 00:00:00 2001 From: Rohith Date: Tue, 31 Jul 2018 21:51:28 +0100 Subject: [PATCH] Requested Changes - changing the name of the command line option to --auto-certificate-annotations - making the option an array to allow for multiple annotations settings Signed-off-by: Rohith Jayawardene --- cmd/controller/app/controller.go | 2 +- cmd/controller/app/options/options.go | 13 +++++++------ pkg/controller/context.go | 4 ++-- pkg/controller/ingress-shim/controller.go | 4 ++-- pkg/controller/ingress-shim/sync.go | 12 +++++++----- pkg/controller/ingress-shim/sync_test.go | 2 +- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index 272d402f4..13887f584 100644 --- a/cmd/controller/app/controller.go +++ b/cmd/controller/app/controller.go @@ -181,7 +181,7 @@ func buildControllerContext(opts *options.ControllerOptions) (*controller.Contex IngressShimOptions: controller.IngressShimOptions{ DefaultIssuerName: opts.DefaultIssuerName, DefaultIssuerKind: opts.DefaultIssuerKind, - DefaultACMEAnnotation: opts.DefaultACMEAnnotation, + DefaultAutoCertificateAnnotations: opts.DefaultAutoCertificateAnnotations, DefaultACMEIssuerChallengeType: opts.DefaultACMEIssuerChallengeType, DefaultACMEIssuerDNS01ProviderName: opts.DefaultACMEIssuerDNS01ProviderName, }, diff --git a/cmd/controller/app/options/options.go b/cmd/controller/app/options/options.go index e0e058fb8..9e6c8dfa5 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -56,9 +56,9 @@ type ControllerOptions struct { RenewBeforeExpiryDuration time.Duration // Default issuer/certificates details consumed by ingress-shim - DefaultACMEAnnotation string - DefaultIssuerName string + DefaultIssuerName string DefaultIssuerKind string + DefaultAutoCertificateAnnotations []string DefaultACMEIssuerChallengeType string DefaultACMEIssuerDNS01ProviderName string @@ -80,9 +80,8 @@ const ( defaultIssuerAmbientCredentials = false defaultRenewBeforeExpiryDuration = time.Hour * 24 * 30 - defaultTLSACMEIssuerName = "" + defaultTLSACMEIssuerName = "" defaultTLSACMEIssuerKind = "Issuer" - defaultACMEAnnotation = "kubernetes.io/tls-acme" defaultACMEIssuerChallengeType = "http01" defaultACMEIssuerDNS01ProviderName = "" ) @@ -94,6 +93,8 @@ var ( defaultACMEHTTP01SolverResourceLimitsCPU = "10m" defaultACMEHTTP01SolverResourceLimitsMemory = "64Mi" + defaultAutoCertificateAnnotations = []string{"kubernetes.io/tls-acme"} + defaultEnabledControllers = []string{ issuerscontroller.ControllerName, clusterissuerscontroller.ControllerName, @@ -117,9 +118,9 @@ func NewControllerOptions() *ControllerOptions { ClusterIssuerAmbientCredentials: defaultClusterIssuerAmbientCredentials, IssuerAmbientCredentials: defaultIssuerAmbientCredentials, RenewBeforeExpiryDuration: defaultRenewBeforeExpiryDuration, - DefaultACMEAnnotation: defaultACMEAnnotation, DefaultIssuerName: defaultTLSACMEIssuerName, DefaultIssuerKind: defaultTLSACMEIssuerKind, + DefaultAutoCertificateAnnotations: defaultAutoCertificateAnnotations, DefaultACMEIssuerChallengeType: defaultACMEIssuerChallengeType, DefaultACMEIssuerDNS01ProviderName: defaultACMEIssuerDNS01ProviderName, DNS01Nameservers: []string{}, @@ -183,7 +184,7 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) { "The default 'renew before expiry' time for Certificates. "+ "Once a certificate is within this duration until expiry, a new Certificate "+ "will be attempted to be issued.") - fs.StringVar(&s.DefaultACMEAnnotation, "default-acme-annotation", defaultACMEAnnotation, ""+ + fs.StringSliceVar(&s.DefaultAutoCertificateAnnotations, "auto-certificate-annotations", defaultAutoCertificateAnnotations, ""+ "The annotation consumed by the ingress-shim controller to indicate a ingress is requesting a certificate") fs.StringVar(&s.DefaultIssuerName, "default-issuer-name", defaultTLSACMEIssuerName, ""+ diff --git a/pkg/controller/context.go b/pkg/controller/context.go index 9ba6feca1..b23d4fcaa 100644 --- a/pkg/controller/context.go +++ b/pkg/controller/context.go @@ -100,9 +100,9 @@ type ACMEOptions struct { type IngressShimOptions struct { // Default issuer/certificates details consumed by ingress-shim - DefaultACMEAnnotation string - DefaultIssuerName string DefaultIssuerKind string + DefaultIssuerName string DefaultACMEIssuerChallengeType string DefaultACMEIssuerDNS01ProviderName string + DefaultAutoCertificateAnnotations []string } diff --git a/pkg/controller/ingress-shim/controller.go b/pkg/controller/ingress-shim/controller.go index 58a023bcc..5db252c27 100644 --- a/pkg/controller/ingress-shim/controller.go +++ b/pkg/controller/ingress-shim/controller.go @@ -46,7 +46,7 @@ const ( ) type defaults struct { - acmeTLSAnnotation string + autoCertificateAnnotations []string issuerName, issuerKind string acmeIssuerChallengeType string acmeIssuerDNS01ProviderName string @@ -218,7 +218,7 @@ func init() { ctx.Client, ctx.CMClient, ctx.Recorder, - defaults{ctx.DefaultACMEAnnotation, ctx.DefaultIssuerName, ctx.DefaultIssuerKind, ctx.DefaultACMEIssuerChallengeType, ctx.DefaultACMEIssuerDNS01ProviderName}, + defaults{ctx.DefaultAutoCertificateAnnotations, ctx.DefaultIssuerName, ctx.DefaultIssuerKind, ctx.DefaultACMEIssuerChallengeType, ctx.DefaultACMEIssuerDNS01ProviderName}, ).Run }) } diff --git a/pkg/controller/ingress-shim/sync.go b/pkg/controller/ingress-shim/sync.go index 65390c916..2d6625b00 100644 --- a/pkg/controller/ingress-shim/sync.go +++ b/pkg/controller/ingress-shim/sync.go @@ -59,7 +59,7 @@ const ( var ingressGVK = extv1beta1.SchemeGroupVersion.WithKind("Ingress") func (c *Controller) Sync(ctx context.Context, ing *extv1beta1.Ingress) error { - if !shouldSync(ing, c.defaults.acmeTLSAnnotation) { + if !shouldSync(ing, c.defaults.autoCertificateAnnotations) { glog.Infof("Not syncing ingress %s/%s as it does not contain necessary annotations", ing.Namespace, ing.Name) return nil } @@ -256,7 +256,7 @@ func (c *Controller) setIssuerSpecificConfig(crt *v1alpha1.Certificate, issuer v // shouldSync returns true if this ingress should have a Certificate resource // created for it -func shouldSync(ing *extv1beta1.Ingress, tlsACMEAnnotation string) bool { +func shouldSync(ing *extv1beta1.Ingress, autoCertificateAnnotations []string) bool { annotations := ing.Annotations if annotations == nil { annotations = map[string]string{} @@ -267,9 +267,11 @@ func shouldSync(ing *extv1beta1.Ingress, tlsACMEAnnotation string) bool { if _, ok := annotations[clusterIssuerNameAnnotation]; ok { return true } - if s, ok := annotations[tlsACMEAnnotation]; ok { - if b, _ := strconv.ParseBool(s); b { - return true + for _, x := range autoCertificateAnnotations { + if s, ok := annotations[x]; ok { + if b, _ := strconv.ParseBool(s); b { + return true + } } } if _, ok := annotations[acmeIssuerChallengeTypeAnnotation]; ok { diff --git a/pkg/controller/ingress-shim/sync_test.go b/pkg/controller/ingress-shim/sync_test.go index 2ec54a4ea..c7dcc4671 100644 --- a/pkg/controller/ingress-shim/sync_test.go +++ b/pkg/controller/ingress-shim/sync_test.go @@ -73,7 +73,7 @@ func TestShouldSync(t *testing.T) { }, } for _, test := range tests { - shouldSync := shouldSync(buildIngress("", "", test.Annotations), "kubernetes.io/tls-acme") + shouldSync := shouldSync(buildIngress("", "", test.Annotations), []string{"kubernetes.io/tls-acme"}) if shouldSync != test.ShouldSync { t.Errorf("Expected shouldSync=%v for annotations %#v", test.ShouldSync, test.Annotations) }