diff --git a/deploy/charts/istio-csr/templates/certificate.yaml b/deploy/charts/istio-csr/templates/certificate.yaml index 968c2898..f4a9b28b 100644 --- a/deploy/charts/istio-csr/templates/certificate.yaml +++ b/deploy/charts/istio-csr/templates/certificate.yaml @@ -1,3 +1,10 @@ +{{- /* + +NOTE: If you change this Certificate remember to also update `pkg/istiodcert`. + This template is designed to statically create the same Certificate resource + that may also be dynamically generated by that Go package. + +*/ -}} {{- if eq (toString .Values.app.tls.istiodCertificateEnable) "true" }} apiVersion: cert-manager.io/v1 kind: Certificate diff --git a/pkg/istiodcert/worker.go b/pkg/istiodcert/worker.go index 2081f7a7..57808835 100644 --- a/pkg/istiodcert/worker.go +++ b/pkg/istiodcert/worker.go @@ -14,6 +14,12 @@ See the License for the specific language governing permissions and limitations under the License. */ +/* +NOTE: If you change this package, remember to also update `deploy/charts/istio-csr/templates/certificate.yaml`. + This package is designed to dynamically create the same Certificate resource + that may also be generated by Helm. +*/ + package istiodcert import ( @@ -239,10 +245,15 @@ func (dicp *DynamicIstiodCertProvisioner) Reconcile(ctx context.Context, req ctr } // makeDNSNamesFromRevisions takes a list of istio revisions and produces a list of -// corresponding DNS names for the istiod cert, as well as returning the value for the common name +// corresponding DNS names for the istiod cert, as well as returning the value for the common name. +// The host label of the non-default DNS names must have the prefix "istiod-", +// because that is the hostname pattern used by clients that connect to the +// Istio Citadel API hosted by Istio CSR. +// This function must produce commonName and dnsNames matching those of the Helm generated certificate, here: +// * https://github.com/cert-manager/istio-csr/blob/main/deploy/charts/istio-csr/templates/certificate.yaml func makeDNSNamesFromRevisions(namespace string, istioRevisions []string) (string, []string) { if len(istioRevisions) == 0 { - // It's unlikely to have no revisisions since the default is to have a + // It's unlikely to have no revisions since the default is to have a // list of revisions containing just "default". // In any case, treat an empty list of revisions as being the same as just // the default revision. @@ -261,7 +272,7 @@ func makeDNSNamesFromRevisions(namespace string, istioRevisions []string) (strin continue } - dnsNames = append(dnsNames, fmt.Sprintf("istiod%s.%s.svc", revision, namespace)) + dnsNames = append(dnsNames, fmt.Sprintf("istiod-%s.%s.svc", revision, namespace)) } // Always return the default SAN as the commonName to match the behaviour of the static istiod cert diff --git a/pkg/istiodcert/worker_test.go b/pkg/istiodcert/worker_test.go new file mode 100644 index 00000000..75dc04d8 --- /dev/null +++ b/pkg/istiodcert/worker_test.go @@ -0,0 +1,95 @@ +/* +Copyright 2024 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package istiodcert + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMakeDNSNamesFromRevisions(t *testing.T) { + type testCase struct { + name string + namespace string + revisions []string + } + + tests := []testCase{ + { + name: "empty-revisions", + namespace: "test1", + }, + { + name: "default-only-revision", + namespace: "test2", + revisions: []string{"default"}, + }, + { + name: "default-first-among-revisions", + namespace: "test3", + revisions: []string{"default", "1-21-0", "1-22-1"}, + }, + { + name: "default-among-revisions", + namespace: "test3", + revisions: []string{"1-21-0", "default", "1-22-1"}, + }, + { + name: "non-default-revisions", + namespace: "test4", + revisions: []string{"1-21-0", "1-22-1"}, + }, + { + name: "duplicate-revisions", + namespace: "test5", + revisions: []string{"default", "1-21-0", "default", "1-22-1", "1-21-0"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + expectedRevisionPrefix := "istiod-" + expectedSuffix := fmt.Sprintf(".%s.svc", tc.namespace) + expectedCommonName := "istiod" + expectedSuffix + + commonName, dnsNames := makeDNSNamesFromRevisions(tc.namespace, tc.revisions) + + assert.Equal(t, expectedCommonName, commonName, + "The commonName should always be istiod..svc") + + foundRevisions := make([]string, len(dnsNames)) + for i, dnsName := range dnsNames { + var revision string + if dnsName == commonName { + revision = "default" + } else { + revision = dnsName[len(expectedRevisionPrefix) : len(dnsName)-len(expectedSuffix)] + } + foundRevisions[i] = revision + } + + if len(tc.revisions) == 0 { + assert.Equal(t, []string{commonName}, dnsNames, + "The dnsNames should contain only the commonName if the supplied revisions list is empty") + } else { + assert.Equal(t, tc.revisions, foundRevisions, + "The original ordered revisions should be recoverable from the DNS names") + } + }) + } +}