Skip to content

Commit b099c2f

Browse files
chiragkyalchiragkyal
authored
CM-681: Utilize istioDataPlaneNamespaceSelector field to control namespace selection (#305)
* Include --configmap-namespace-selector arg into istio-csr deployment Signed-off-by: chiragkyal <[email protected]> * Add e2e for istioDataPlaneNamespaceSelector functionality Signed-off-by: chiragkyal <[email protected]> * Address review comments Signed-off-by: chiragkyal <[email protected]> --------- Signed-off-by: chiragkyal <[email protected]>
1 parent 93dc342 commit b099c2f

File tree

7 files changed

+260
-49
lines changed

7 files changed

+260
-49
lines changed

‎pkg/controller/istiocsr/deployments.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ func updateArgList(deployment *appsv1.Deployment, istiocsr *v1alpha1.IstioCSR) {
168168
fmt.Sprintf("--istiod-cert-istio-revisions=%s", strings.Join(istiocsrConfigs.Istio.Revisions, ",")),
169169
}
170170

171+
// Add configmap-namespace-selector argument if configured
172+
if istiocsrConfigs.IstioDataPlaneNamespaceSelector != "" {
173+
args = append(args, fmt.Sprintf("--configmap-namespace-selector=%s", istiocsrConfigs.IstioDataPlaneNamespaceSelector))
174+
}
175+
171176
for i, container := range deployment.Spec.Template.Spec.Containers {
172177
if container.Name == istiocsrContainerName {
173178
deployment.Spec.Template.Spec.Containers[i].Args = args

‎pkg/controller/istiocsr/deployments_test.go‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"slices"
7+
"strings"
78
"testing"
89

910
appsv1 "k8s.io/api/apps/v1"
@@ -741,9 +742,10 @@ func TestCreateOrApplyDeployments(t *testing.T) {
741742

742743
func TestUpdateArgList(t *testing.T) {
743744
tests := []struct {
744-
name string
745-
updateIstioCSR func(*v1alpha1.IstioCSR)
746-
expectedArgs map[string]string // key is arg name (without --), value is expected value
745+
name string
746+
updateIstioCSR func(*v1alpha1.IstioCSR)
747+
expectedArgs map[string]string // key is arg name (without --), value is expected value
748+
notExpectedArgs []string // arg names (without --) that should NOT be present
747749
}{
748750
{
749751
name: "clusterID not provided should default to Kubernetes",
@@ -776,6 +778,21 @@ func TestUpdateArgList(t *testing.T) {
776778
"cluster-id": "cluster-123_dev.local",
777779
},
778780
},
781+
{
782+
name: "istioDataPlaneNamespaceSelector not provided should not include argument",
783+
notExpectedArgs: []string{
784+
"configmap-namespace-selector",
785+
},
786+
},
787+
{
788+
name: "istioDataPlaneNamespaceSelector provided should include argument",
789+
updateIstioCSR: func(istiocsr *v1alpha1.IstioCSR) {
790+
istiocsr.Spec.IstioCSRConfig.IstioDataPlaneNamespaceSelector = "cert-manager.io/test-ca-injection=enabled"
791+
},
792+
expectedArgs: map[string]string{
793+
"configmap-namespace-selector": "cert-manager.io/test-ca-injection=enabled",
794+
},
795+
},
779796
}
780797

781798
for _, tt := range tests {
@@ -808,6 +825,16 @@ func TestUpdateArgList(t *testing.T) {
808825
t.Errorf("Expected to find argument %q in container args, but it was not found. Args: %v", expectedArg, containerArgs)
809826
}
810827
}
828+
829+
// Verify arguments that should NOT be present
830+
for _, argName := range tt.notExpectedArgs {
831+
for _, arg := range containerArgs {
832+
if strings.HasPrefix(arg, fmt.Sprintf("--%s=", argName)) {
833+
t.Errorf("Expected NOT to find argument %q in container args. Args: %v", arg, containerArgs)
834+
break
835+
}
836+
}
837+
}
811838
})
812839
}
813840
}

‎test/e2e/config_template.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type CertificateConfig struct {
2121
DNSName string
2222
}
2323

24-
// IstioCSRConfig customizes the fields in a job spec
24+
// IstioCSRGRPCurlJobConfig customizes the fields in a job spec
2525
type IstioCSRGRPCurlJobConfig struct {
2626
CertificateSigningRequest string
2727
IstioCSRStatus v1alpha1.IstioCSRStatus
@@ -31,7 +31,7 @@ type IstioCSRGRPCurlJobConfig struct {
3131

3232
// replaceWithTemplate puts field values from a template struct
3333
func replaceWithTemplate(sourceFileContents string, templatedValues any) ([]byte, error) {
34-
tmpl, err := template.New("template").Parse(sourceFileContents)
34+
tmpl, err := template.New("template").Option("missingkey=error").Parse(sourceFileContents)
3535
if err != nil {
3636
return nil, err
3737
}

‎test/e2e/istio_csr_test.go‎

Lines changed: 175 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"encoding/json"
1111
"fmt"
1212
"io"
13+
"log"
1314
"net/url"
1415
"path/filepath"
1516

@@ -35,7 +36,8 @@ type LogEntry struct {
3536
}
3637

3738
type IstioCSRConfig struct {
38-
ClusterID string
39+
ClusterID string
40+
IstioDataPlaneNamespaceSelector string
3941
}
4042

4143
var _ = Describe("Istio-CSR", Ordered, Label("Feature:IstioCSR"), func() {
@@ -98,6 +100,27 @@ var _ = Describe("Istio-CSR", Ordered, Label("Feature:IstioCSR"), func() {
98100
Expect(err).NotTo(HaveOccurred())
99101
ns = namespace
100102

103+
By("creating cluster issuer")
104+
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "cluster_issuer.yaml"), ns.Name)
105+
DeferCleanup(func() {
106+
loader.DeleteFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "cluster_issuer.yaml"), ns.Name)
107+
})
108+
109+
By("issuing TLS certificate")
110+
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "certificate.yaml"), ns.Name)
111+
DeferCleanup(func() {
112+
loader.DeleteFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "certificate.yaml"), ns.Name)
113+
})
114+
115+
err = waitForCertificateReadiness(ctx, "my-selfsigned-ca", ns.Name)
116+
Expect(err).NotTo(HaveOccurred())
117+
118+
By("creating istio-ca issuer")
119+
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "istio", "istio_ca_issuer.yaml"), ns.Name)
120+
DeferCleanup(func() {
121+
loader.DeleteFromFile(testassets.ReadFile, filepath.Join("testdata", "istio", "istio_ca_issuer.yaml"), ns.Name)
122+
})
123+
101124
DeferCleanup(func() {
102125
By("deleting cluster-scoped RBAC resources of the istio-csr agent")
103126
clientset.RbacV1().ClusterRoles().DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{
@@ -114,21 +137,6 @@ var _ = Describe("Istio-CSR", Ordered, Label("Feature:IstioCSR"), func() {
114137

115138
Context("grpc call istio.v1.auth.IstioCertificateService/CreateCertificate to istio-csr agent", func() {
116139
BeforeEach(func() {
117-
By("creating cluster issuer")
118-
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "cluster_issuer.yaml"), ns.Name)
119-
DeferCleanup(func() {
120-
loader.DeleteFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "cluster_issuer.yaml"), ns.Name)
121-
})
122-
123-
By("issuing TLS certificate")
124-
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "certificate.yaml"), ns.Name)
125-
DeferCleanup(func() {
126-
loader.DeleteFromFile(testassets.ReadFile, filepath.Join("testdata", "self_signed", "certificate.yaml"), ns.Name)
127-
})
128-
129-
err := waitForCertificateReadiness(ctx, "my-selfsigned-ca", ns.Name)
130-
Expect(err).NotTo(HaveOccurred())
131-
132140
By("fetching proto file from api")
133141
protoContent, err := library.FetchFileFromURL(istioCSRProtoURL)
134142
Expect(err).Should(BeNil())
@@ -149,21 +157,19 @@ var _ = Describe("Istio-CSR", Ordered, Label("Feature:IstioCSR"), func() {
149157
DeferCleanup(func() {
150158
clientset.CoreV1().ConfigMaps(ns.Name).Delete(ctx, configMap.Name, metav1.DeleteOptions{})
151159
})
152-
153-
By("creating istio-ca issuer")
154-
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "istio", "istio_ca_issuer.yaml"), ns.Name)
155-
DeferCleanup(func() {
156-
loader.DeleteFromFile(testassets.ReadFile, filepath.Join("testdata", "istio", "istio_ca_issuer.yaml"), ns.Name)
157-
})
158160
})
159161

160162
It("should return cert-chain as response", func() {
161163
serviceAccountName := "cert-manager-istio-csr"
162164
grpcAppName := "grpcurl-istio-csr"
163165

164166
By("creating istiocsr.operator.openshift.io resource")
165-
loader.CreateFromFile(testassets.ReadFile, filepath.Join("testdata", "istio", "istio_csr.yaml"), ns.Name)
166-
defer loader.DeleteFromFile(testassets.ReadFile, filepath.Join("testdata", "istio", "istio_csr.yaml"), ns.Name)
167+
loader.CreateFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
168+
IstioCSRConfig{},
169+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
170+
defer loader.DeleteFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
171+
IstioCSRConfig{},
172+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
167173

168174
istioCSRStatus := waitForIstioCSRReady(ns)
169175

@@ -231,12 +237,12 @@ var _ = Describe("Istio-CSR", Ordered, Label("Feature:IstioCSR"), func() {
231237
IstioCSRConfig{
232238
ClusterID: clusterName,
233239
},
234-
), filepath.Join("testdata", "istio", "istio_csr_custom_cluster_id.yaml"), ns.Name)
240+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
235241
defer loader.DeleteFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
236242
IstioCSRConfig{
237243
ClusterID: clusterName,
238244
},
239-
), filepath.Join("testdata", "istio", "istio_csr_custom_cluster_id.yaml"), ns.Name)
245+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
240246

241247
istioCSRStatus := waitForIstioCSRReady(ns)
242248

@@ -299,12 +305,12 @@ var _ = Describe("Istio-CSR", Ordered, Label("Feature:IstioCSR"), func() {
299305
IstioCSRConfig{
300306
ClusterID: clusterName,
301307
},
302-
), filepath.Join("testdata", "istio", "istio_csr_custom_cluster_id.yaml"), ns.Name)
308+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
303309
defer loader.DeleteFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
304310
IstioCSRConfig{
305311
ClusterID: clusterName,
306312
},
307-
), filepath.Join("testdata", "istio", "istio_csr_custom_cluster_id.yaml"), ns.Name)
313+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
308314

309315
istioCSRStatus := waitForIstioCSRReady(ns)
310316

@@ -348,4 +354,145 @@ var _ = Describe("Istio-CSR", Ordered, Label("Feature:IstioCSR"), func() {
348354
Expect(succeededPodName).Should(BeEmpty(), "Job should have failed due to wrong clusterID")
349355
})
350356
})
357+
358+
Context("istioDataPlaneNamespaceSelector functionality", func() {
359+
var testNamespaces []*corev1.Namespace
360+
361+
BeforeEach(func() {
362+
By("creating test namespaces with different labels")
363+
namespacesToCreate := []*corev1.Namespace{
364+
{
365+
ObjectMeta: metav1.ObjectMeta{
366+
GenerateName: "ca-injection-enabled-",
367+
Labels: map[string]string{
368+
"cert-manager.io/test-ca-injection": "enabled", // matches the IstioCSR resource
369+
},
370+
},
371+
},
372+
{
373+
ObjectMeta: metav1.ObjectMeta{
374+
GenerateName: "ca-injection-disabled-",
375+
Labels: map[string]string{
376+
"cert-manager.io/test-ca-injection": "disabled", // doesn't match the IstioCSR resource
377+
},
378+
},
379+
},
380+
{
381+
ObjectMeta: metav1.ObjectMeta{
382+
GenerateName: "ca-injection-unlabeled-",
383+
// no labels
384+
},
385+
},
386+
}
387+
388+
for _, ns := range namespacesToCreate {
389+
createdNs, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{})
390+
Expect(err).Should(BeNil())
391+
testNamespaces = append(testNamespaces, createdNs)
392+
}
393+
394+
DeferCleanup(func() {
395+
for _, testNs := range testNamespaces {
396+
err := clientset.CoreV1().Namespaces().Delete(ctx, testNs.Name, metav1.DeleteOptions{})
397+
Expect(err).Should(BeNil())
398+
}
399+
testNamespaces = []*corev1.Namespace{}
400+
})
401+
})
402+
403+
It("should only create istio-ca-root-cert ConfigMap in namespaces matching the selector", func() {
404+
By("creating istiocsr.operator.openshift.io resource with istioDataPlaneNamespaceSelector")
405+
loader.CreateFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
406+
IstioCSRConfig{
407+
IstioDataPlaneNamespaceSelector: "cert-manager.io/test-ca-injection=enabled",
408+
},
409+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
410+
defer loader.DeleteFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
411+
IstioCSRConfig{
412+
IstioDataPlaneNamespaceSelector: "cert-manager.io/test-ca-injection=enabled",
413+
},
414+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
415+
416+
By("waiting for IstioCSR to be ready and deployment to be created")
417+
istioCSRStatus := waitForIstioCSRReady(ns)
418+
log.Printf("IstioCSR status: %+v", istioCSRStatus)
419+
420+
By("verifying ConfigMap creation based on namespace selector")
421+
var matchingNs *corev1.Namespace
422+
for _, testNs := range testNamespaces {
423+
labelValue, hasLabel := testNs.Labels["cert-manager.io/test-ca-injection"]
424+
425+
if hasLabel && labelValue == "enabled" {
426+
// This namespace should have the ConfigMap
427+
matchingNs = testNs
428+
err := pollTillConfigMapAvailable(ctx, clientset, testNs.Name, "istio-ca-root-cert")
429+
Expect(err).Should(BeNil(), fmt.Sprintf("ConfigMap should exist in namespace %s (enabled)", testNs.Name))
430+
} else {
431+
// This namespace should NOT have the ConfigMap
432+
var reason string
433+
if !hasLabel {
434+
reason = "unlabeled"
435+
} else {
436+
reason = fmt.Sprintf("with %s label", labelValue)
437+
}
438+
439+
err := pollTillConfigMapRemains(ctx, clientset, testNs.Name, "istio-ca-root-cert", lowTimeout)
440+
Expect(err).Should(BeNil(), fmt.Sprintf("ConfigMap should NOT exist in namespace %s (%s)", testNs.Name, reason))
441+
}
442+
}
443+
Expect(matchingNs).NotTo(BeNil(), "Should have found at least one namespace with enabled label")
444+
445+
By("verifying the ConfigMap contains the correct CA certificate data")
446+
cm, err := clientset.CoreV1().ConfigMaps(matchingNs.Name).Get(ctx, "istio-ca-root-cert", metav1.GetOptions{})
447+
Expect(err).Should(BeNil())
448+
Expect(cm.Data).Should(HaveKey("root-cert.pem"))
449+
Expect(cm.Data["root-cert.pem"]).ShouldNot(BeEmpty())
450+
451+
By("verifying ConfigMap exists in exactly 1 namespace (the one with matching label)")
452+
configMapCount := 0
453+
for _, testNs := range testNamespaces {
454+
_, err := clientset.CoreV1().ConfigMaps(testNs.Name).Get(ctx, "istio-ca-root-cert", metav1.GetOptions{})
455+
if err == nil {
456+
configMapCount++
457+
}
458+
}
459+
Expect(configMapCount).Should(Equal(1), "ConfigMap should exist in exactly 1 namespace (the one with enabled label)")
460+
})
461+
462+
It("should create istio-ca-root-cert ConfigMap in all namespaces when istioDataPlaneNamespaceSelector is not set", func() {
463+
By("creating istiocsr.operator.openshift.io resource without istioDataPlaneNamespaceSelector")
464+
loader.CreateFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
465+
IstioCSRConfig{},
466+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
467+
defer loader.DeleteFromFile(AssetFunc(testassets.ReadFile).WithTemplateValues(
468+
IstioCSRConfig{},
469+
), filepath.Join("testdata", "istio", "istio_csr_template.yaml"), ns.Name)
470+
471+
By("waiting for IstioCSR to be ready and deployment to be created")
472+
istioCSRStatus := waitForIstioCSRReady(ns)
473+
log.Printf("IstioCSR status: %+v", istioCSRStatus)
474+
475+
By("waiting for istio-ca-root-cert ConfigMap to be created in all test namespaces")
476+
for _, testNs := range testNamespaces {
477+
err := pollTillConfigMapAvailable(ctx, clientset, testNs.Name, "istio-ca-root-cert")
478+
Expect(err).Should(BeNil(), fmt.Sprintf("ConfigMap should be created in namespace %s", testNs.Name))
479+
480+
By(fmt.Sprintf("verifying ConfigMap content in namespace %s", testNs.Name))
481+
cm, err := clientset.CoreV1().ConfigMaps(testNs.Name).Get(ctx, "istio-ca-root-cert", metav1.GetOptions{})
482+
Expect(err).Should(BeNil())
483+
Expect(cm.Data).Should(HaveKey("root-cert.pem"))
484+
Expect(cm.Data["root-cert.pem"]).ShouldNot(BeEmpty())
485+
}
486+
487+
By("verifying ConfigMap exists in all 3 test namespaces when no selector is used")
488+
configMapCount := 0
489+
for _, testNs := range testNamespaces {
490+
_, err := clientset.CoreV1().ConfigMaps(testNs.Name).Get(ctx, "istio-ca-root-cert", metav1.GetOptions{})
491+
if err == nil {
492+
configMapCount++
493+
}
494+
}
495+
Expect(configMapCount).Should(Equal(3), "ConfigMap should exist in all 3 test namespaces when no selector is configured")
496+
})
497+
})
351498
})

‎test/e2e/testdata/istio/istio_csr.yaml‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎test/e2e/testdata/istio/istio_csr_custom_cluster_id.yaml‎ renamed to ‎test/e2e/testdata/istio/istio_csr_template.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,10 @@ spec:
1414
trustDomain: cluster.local
1515
istio:
1616
namespace: istio-system
17+
{{- if .ClusterID}}
1718
server:
1819
clusterID: {{.ClusterID}}
20+
{{- end}}
21+
{{- if .IstioDataPlaneNamespaceSelector}}
22+
istioDataPlaneNamespaceSelector: "{{.IstioDataPlaneNamespaceSelector}}"
23+
{{- end}}

0 commit comments

Comments
 (0)