From 60c56a16ab54aff1bb2ecbd7d8dd5f2bf77aebee Mon Sep 17 00:00:00 2001 From: zzjin Date: Mon, 8 Jul 2024 19:49:51 +0800 Subject: [PATCH] Support `CredentialConfig.TLSSecret` with namespace. Resolve: #1066 (#1095) Signed-off-by: zzjin --- .../templates/controller-clusterrole.yaml | 6 +-- pkg/cert/config.go | 19 ++++++++- pkg/cert/config_test.go | 33 ++++++++++++++++ pkg/cert/secret.go | 39 +++++++------------ pkg/ingress/kube/ingress/controller.go | 10 +++++ pkg/ingress/kube/ingressv1/controller.go | 10 +++++ 6 files changed, 89 insertions(+), 28 deletions(-) diff --git a/helm/core/templates/controller-clusterrole.yaml b/helm/core/templates/controller-clusterrole.yaml index e67baa672..d288945d6 100644 --- a/helm/core/templates/controller-clusterrole.yaml +++ b/helm/core/templates/controller-clusterrole.yaml @@ -36,7 +36,7 @@ rules: # Needed for multicluster secret reading, possibly ingress certs in the future - apiGroups: [""] resources: ["secrets"] - verbs: ["get", "watch", "list"] + verbs: ["get", "watch", "list", "create", "update", "delete", "patch"] - apiGroups: ["networking.higress.io"] resources: ["mcpbridges"] @@ -66,7 +66,7 @@ rules: - apiGroups: ["discovery.k8s.io"] resources: ["endpointslices"] verbs: ["get", "list", "watch"] - + # Istiod and bootstrap. - apiGroups: ["certificates.k8s.io"] resources: @@ -100,7 +100,7 @@ rules: - apiGroups: ["multicluster.x-k8s.io"] resources: ["serviceimports"] verbs: ["get", "watch", "list"] - + # sidecar injection controller - apiGroups: ["admissionregistration.k8s.io"] resources: ["mutatingwebhookconfigurations"] diff --git a/pkg/cert/config.go b/pkg/cert/config.go index 1552ce9be..4ddeec27d 100644 --- a/pkg/cert/config.go +++ b/pkg/cert/config.go @@ -86,6 +86,17 @@ func (c *Config) GetSecretNameByDomain(issuerName IssuerName, domain string) str return "" } +func ParseTLSSecret(tlsSecret string) (string, string) { + secrets := strings.Split(tlsSecret, "/") + switch len(secrets) { + case 1: + return "", tlsSecret + case 2: + return secrets[0], secrets[1] + } + return "", "" +} + func (c *Config) Validate() error { // check acmeIssuer if len(c.ACMEIssuer) == 0 { @@ -111,14 +122,20 @@ func (c *Config) Validate() error { } if credential.TLSSecret == "" { return fmt.Errorf("credentialConfig tlsSecret is empty") + } else { + ns, secret := ParseTLSSecret(credential.TLSSecret) + if ns == "" && secret == "" { + return fmt.Errorf("credentialConfig tlsSecret %s is not supported", credential.TLSSecret) + } } + if credential.TLSIssuer == IssuerTypeLetsencrypt { if len(credential.Domains) > 1 { return fmt.Errorf("credentialConfig tlsIssuer %s only support one domain", credential.TLSIssuer) } } if credential.TLSIssuer != IssuerTypeLetsencrypt && len(credential.TLSIssuer) > 0 { - return fmt.Errorf("credential tls issuer %s is not support", credential.TLSIssuer) + return fmt.Errorf("credential tls issuer %s is not supported", credential.TLSIssuer) } } diff --git a/pkg/cert/config_test.go b/pkg/cert/config_test.go index 0a802e9e8..ef303cf07 100644 --- a/pkg/cert/config_test.go +++ b/pkg/cert/config_test.go @@ -120,3 +120,36 @@ func TestMatchSecretNameByDomain(t *testing.T) { }) } } + +func TestParseTLSSecret(t *testing.T) { + tests := []struct { + tlsSecret string + expectedNamespace string + expectedSecretName string + }{ + { + tlsSecret: "example-com-tls", + expectedNamespace: "", + expectedSecretName: "example-com-tls", + }, + + { + tlsSecret: "kube-system/example-com-tls", + expectedNamespace: "kube-system", + expectedSecretName: "example-com-tls", + }, + { + tlsSecret: "kube-system/example-com/wildcard", + expectedNamespace: "", + expectedSecretName: "", + }, + } + + for _, tt := range tests { + t.Run(tt.tlsSecret, func(t *testing.T) { + resultNamespace, resultSecretName := ParseTLSSecret(tt.tlsSecret) + assert.Equal(t, tt.expectedNamespace, resultNamespace) + assert.Equal(t, tt.expectedSecretName, resultSecretName) + }) + } +} diff --git a/pkg/cert/secret.go b/pkg/cert/secret.go index 0bfac2640..a8b7dcade 100644 --- a/pkg/cert/secret.go +++ b/pkg/cert/secret.go @@ -18,7 +18,6 @@ import ( "context" "fmt" "strconv" - "strings" "time" v1 "k8s.io/api/core/v1" @@ -27,10 +26,6 @@ import ( "k8s.io/client-go/kubernetes" ) -const ( - SecretNamePrefix = "higress-secret-" -) - type SecretMgr struct { client kubernetes.Interface namespace string @@ -46,13 +41,20 @@ func NewSecretMgr(namespace string, client kubernetes.Interface) (*SecretMgr, er } func (s *SecretMgr) Update(domain string, secretName string, privateKey []byte, certificate []byte, notBefore time.Time, notAfter time.Time, isRenew bool) error { - //secretName := s.getSecretName(domain) - secret := s.constructSecret(domain, privateKey, certificate, notBefore, notAfter, isRenew) - _, err := s.client.CoreV1().Secrets(s.namespace).Get(context.Background(), secretName, metav1.GetOptions{}) + name := secretName + namespace := s.namespace + namespaceP, secretP := ParseTLSSecret(secretName) + if namespaceP != "" { + namespace = namespaceP + name = secretP + } + + secret := s.constructSecret(domain, name, namespace, privateKey, certificate, notBefore, notAfter, isRenew) + _, err := s.client.CoreV1().Secrets(namespace).Get(context.Background(), name, metav1.GetOptions{}) if err != nil { if errors.IsNotFound(err) { // create secret - _, err2 := s.client.CoreV1().Secrets(s.namespace).Create(context.Background(), secret, metav1.CreateOptions{}) + _, err2 := s.client.CoreV1().Secrets(namespace).Create(context.Background(), secret, metav1.CreateOptions{}) return err2 } return err @@ -61,7 +63,7 @@ func (s *SecretMgr) Update(domain string, secretName string, privateKey []byte, if _, ok := secret.Annotations["higress.io/cert-domain"]; !ok { return fmt.Errorf("the secret name %s is not automatic https secret name for the domain:%s, please rename it in config", secretName, domain) } - _, err1 := s.client.CoreV1().Secrets(s.namespace).Update(context.Background(), secret, metav1.UpdateOptions{}) + _, err1 := s.client.CoreV1().Secrets(namespace).Update(context.Background(), secret, metav1.UpdateOptions{}) if err1 != nil { return err1 } @@ -69,18 +71,7 @@ func (s *SecretMgr) Update(domain string, secretName string, privateKey []byte, return nil } -func (s *SecretMgr) Delete(domain string) error { - secretName := s.getSecretName(domain) - err := s.client.CoreV1().Secrets(s.namespace).Delete(context.Background(), secretName, metav1.DeleteOptions{}) - return err -} - -func (s *SecretMgr) getSecretName(domain string) string { - return SecretNamePrefix + strings.ReplaceAll(strings.TrimSpace(domain), ".", "-") -} - -func (s *SecretMgr) constructSecret(domain string, privateKey []byte, certificate []byte, notBefore time.Time, notAfter time.Time, isRenew bool) *v1.Secret { - secretName := s.getSecretName(domain) +func (s *SecretMgr) constructSecret(domain string, name string, namespace string, privateKey []byte, certificate []byte, notBefore time.Time, notAfter time.Time, isRenew bool) *v1.Secret { annotationMap := make(map[string]string, 0) annotationMap["higress.io/cert-domain"] = domain annotationMap["higress.io/cert-notAfter"] = notAfter.Format("2006-01-02 15:04:05") @@ -97,8 +88,8 @@ func (s *SecretMgr) constructSecret(domain string, privateKey []byte, certificat dataMap["tls.crt"] = certificate secret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - Namespace: s.namespace, + Name: name, + Namespace: namespace, Annotations: annotationMap, }, Type: v1.SecretTypeTLS, diff --git a/pkg/ingress/kube/ingress/controller.go b/pkg/ingress/kube/ingress/controller.go index 521870e23..32a5cd2b7 100644 --- a/pkg/ingress/kube/ingress/controller.go +++ b/pkg/ingress/kube/ingress/controller.go @@ -433,6 +433,11 @@ func (c *controller) ConvertGateway(convertOptions *common.ConvertOptions, wrapp // If there is no matching secret, try to get it from configmap. secretName = httpsCredentialConfig.MatchSecretNameByDomain(rule.Host) secretNamespace = c.options.SystemNamespace + namespace, secret := cert.ParseTLSSecret(secretName) + if namespace != "" { + secretNamespace = namespace + secretName = secret + } } } } @@ -441,6 +446,11 @@ func (c *controller) ConvertGateway(convertOptions *common.ConvertOptions, wrapp if httpsCredentialConfig != nil { secretName = httpsCredentialConfig.MatchSecretNameByDomain(rule.Host) secretNamespace = c.options.SystemNamespace + namespace, secret := cert.ParseTLSSecret(secretName) + if namespace != "" { + secretNamespace = namespace + secretName = secret + } } } if secretName == "" { diff --git a/pkg/ingress/kube/ingressv1/controller.go b/pkg/ingress/kube/ingressv1/controller.go index f13d19385..b15e4b198 100644 --- a/pkg/ingress/kube/ingressv1/controller.go +++ b/pkg/ingress/kube/ingressv1/controller.go @@ -419,6 +419,11 @@ func (c *controller) ConvertGateway(convertOptions *common.ConvertOptions, wrapp // If there is no matching secret, try to get it from configmap. secretName = httpsCredentialConfig.MatchSecretNameByDomain(rule.Host) secretNamespace = c.options.SystemNamespace + namespace, secret := cert.ParseTLSSecret(secretName) + if namespace != "" { + secretNamespace = namespace + secretName = secret + } } } } @@ -427,6 +432,11 @@ func (c *controller) ConvertGateway(convertOptions *common.ConvertOptions, wrapp if httpsCredentialConfig != nil { secretName = httpsCredentialConfig.MatchSecretNameByDomain(rule.Host) secretNamespace = c.options.SystemNamespace + namespace, secret := cert.ParseTLSSecret(secretName) + if namespace != "" { + secretNamespace = namespace + secretName = secret + } } }