CertSAN added as a patch json in clusterclass patches #372

deepakm-ntnx · 2024-01-24T23:53:17Z

What this PR does / why we need it:
Makes certSAN for apiServer configurable as variable in cluster with topology

certSANs[]string | certSANs sets extra Subject Alternative Names (SANs) for the API Server signing certificate.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

How Has This Been Tested?:

make test-e2e-calico LABEL_FILTERS=quickstart

When following the Cluster API quick-start with ClusterClass Should create a workload cluster [quickstart2, capx-feature-test]
/Users/deepak.muley/go/src/github.com/kubernetes-sigs/cluster-api/test/e2e/quick_start.go:78
  STEP: Creating a namespace for hosting the "quick-start" test spec @ 01/25/24 15:47:37.402
  INFO: Creating namespace quick-start-1v7ula
  INFO: Creating event watcher for namespace "quick-start-1v7ula"
  STEP: Creating a workload cluster @ 01/25/24 15:47:37.43
  INFO: Creating the workload cluster with name "quick-start-kt0qgz" using the "topology" template (Kubernetes v1.27.9, 1 control-plane machines, 1 worker machines)
  INFO: Getting the cluster template yaml
  INFO: clusterctl config cluster quick-start-kt0qgz --infrastructure (default) --kubernetes-version v1.27.9 --control-plane-machine-count 1 --worker-machine-count 1 --flavor topology
  INFO: Applying the cluster template yaml to the cluster
configmap/quick-start-kt0qgz-pc-trusted-ca-bundle created
configmap/nutanix-ccm created
secret/quick-start-kt0qgz created
secret/nutanix-ccm-secret created
clusterresourceset.addons.cluster.x-k8s.io/nutanix-ccm-crs created
kubeadmconfigtemplate.bootstrap.cluster.x-k8s.io/quick-start-kt0qgz-kcfg-0 created
clusterclass.cluster.x-k8s.io/e2e created
kubeadmcontrolplanetemplate.controlplane.cluster.x-k8s.io/e2e-kcpt created
nutanixclustertemplate.infrastructure.cluster.x-k8s.io/e2e-nct created
nutanixmachinetemplate.infrastructure.cluster.x-k8s.io/e2e-cp-nmt created
nutanixmachinetemplate.infrastructure.cluster.x-k8s.io/e2e-md-nmt created
configmap/cni-quick-start-kt0qgz-crs-cni created
clusterresourceset.addons.cluster.x-k8s.io/quick-start-kt0qgz-crs-cni created
cluster.cluster.x-k8s.io/quick-start-kt0qgz created

  INFO: Waiting for the cluster infrastructure to be provisioned
  STEP: Waiting for cluster to enter the provisioned phase @ 01/25/24 15:47:41.458
  INFO: Waiting for control plane to be initialized
  INFO: Waiting for the first control plane machine managed by quick-start-1v7ula/quick-start-kt0qgz-fw44r to be provisioned
  STEP: Waiting for one control plane node to exist @ 01/25/24 15:47:51.539
  INFO: Waiting for control plane to be ready
  INFO: Waiting for control plane quick-start-1v7ula/quick-start-kt0qgz-fw44r to be ready (implies underlying nodes to be ready as well)
  STEP: Waiting for the control plane to be ready @ 01/25/24 15:48:51.669
  STEP: Checking all the control plane machines are in the expected failure domains @ 01/25/24 15:49:01.685
  INFO: Waiting for the machine deployments to be provisioned
  STEP: Waiting for the workload nodes to exist @ 01/25/24 15:49:01.717
  STEP: Checking all the machines controlled by quick-start-kt0qgz-md-0-lfxr9 are in the "" failure domain @ 01/25/24 15:49:21.757
  INFO: Waiting for the machine pools to be provisioned
  STEP: PASSED! @ 01/25/24 15:49:21.845
  STEP: Dumping logs from the "quick-start-kt0qgz" workload cluster @ 01/25/24 15:49:21.845
Failed to get logs for Machine quick-start-kt0qgz-fw44r-l77sj, Cluster quick-start-1v7ula/quick-start-kt0qgz: error creating container exec: Error response from daemon: No such container: quick-start-kt0qgz-fw44r-l77sj
Failed to get logs for Machine quick-start-kt0qgz-md-0-lfxr9-pv6q9-8r88f, Cluster quick-start-1v7ula/quick-start-kt0qgz: error creating container exec: Error response from daemon: No such container: quick-start-kt0qgz-md-0-lfxr9-pv6q9-8r88f
  STEP: Dumping all the Cluster API resources in the "quick-start-1v7ula" namespace @ 01/25/24 15:49:22.008
  STEP: Deleting cluster quick-start-1v7ula/quick-start-kt0qgz @ 01/25/24 15:49:22.243
  STEP: Deleting cluster quick-start-kt0qgz @ 01/25/24 15:49:22.272
  INFO: Waiting for the Cluster quick-start-1v7ula/quick-start-kt0qgz to be deleted
  STEP: Waiting for cluster quick-start-kt0qgz to be deleted @ 01/25/24 15:49:22.285
  STEP: Deleting namespace used for hosting the "quick-start" test spec @ 01/25/24 15:49:52.311
  INFO: Deleting namespace quick-start-1v7ula
• [134.955 seconds]

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration and test output

Special notes for your reviewer:
TODO:

trying to figure out if we need to make this into a variable with default value so that use can pass more entries as a bulk as array appending is not possible

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

codecov · 2024-01-24T23:59:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.24%. Comparing base (c075eb4) to head (26cba45).
Report is 58 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #372   +/-   ##
=======================================
  Coverage   15.24%   15.24%           
=======================================
  Files          18       18           
  Lines        1207     1207           
=======================================
  Hits          184      184           
  Misses       1023     1023

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

thunderboltsid · 2024-01-26T10:52:35Z

/retest

thunderboltsid · 2024-01-26T14:49:13Z

templates/cluster-template-clusterclass.yaml

+      openAPIV3Schema:
+        description: Set extra Subject Alternative Names (SANs) for the API Server
+          signing certificate.
+        type: string


this should be a list of strings

wanted to avoid any issue as mentioned in kubernetes-sigs/cluster-api#6245

thunderboltsid · 2024-01-26T14:49:38Z

templates/cluster-template-clusterclass.yaml

+        path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer
+        valueFrom:
+          template: |
+            certSANs: [ {{ .apiServerSigningCertExtraCertSANs }} ]


this should just be {{ .apiServerSigningCertExtraCertSANs }}

could you please elaborate why it should be? it works current way

thunderboltsid · 2024-01-26T14:50:42Z

templates/topology/cluster-with-topology.yaml

+        subnetName: "${NUTANIX_SUBNET_NAME}"
+    - name: apiServerSigningCertExtraCertSANs
+      value: "localhost, 127.0.0.1, 0.0.0.0"


this should be a list of strings rather than a CSV string

thunderboltsid · 2024-01-26T14:51:52Z

templates/cluster-template-topology.yaml

@@ -52,6 +52,8 @@ spec:
        systemDiskSize: ${NUTANIX_SYSTEMDISK_SIZE=40Gi}
        vcpuSockets: ${NUTANIX_MACHINE_VCPU_SOCKET=2}
        vcpusPerSocket: ${NUTANIX_MACHINE_VCPU_PER_SOCKET=1}
+    - name: apiServerSigningCertExtraCertSANs
+      value: localhost, 127.0.0.1, 0.0.0.0


this should be a list of strings rather than a CSV string

thunderboltsid · 2024-01-26T14:53:04Z

templates/clusterclass/clusterclass.yaml

+    schema:
+      openAPIV3Schema:
+        description: Set extra Subject Alternative Names (SANs) for the API Server signing certificate.
+        type: string


this should be a list of strings

thunderboltsid · 2024-01-26T14:53:23Z

templates/clusterclass/clusterclass.yaml

+        path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer
+        valueFrom:
+          template: |
+            certSANs: [ {{ .apiServerSigningCertExtraCertSANs }} ]


this should just be {{ .apiServerSigningCertExtraCertSANs }}

reason: it was carry forward from docker provider and is not needed for nutanix provider kcp and kcpt

dkoshkin · 2024-01-27T05:24:34Z

templates/cluster-template-clusterclass.yaml

+      - op: add
+        path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer
+        valueFrom:
+          template: |
+            certSANs: [ {{ .apiServerSigningCertExtraCertSANs }} ]


This is the limitation of inline patches and the difficulty of testing it :(
Having path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer will remove all other options in apiServer both from templates and set by previously applied patches.

I would suggest something like this (this is already being tested in DKP in e2e tests). This initializes an empty array and then sets certSANs if apiServerSigningCertExtraCertSANs is not empty. Please also note this includes new definitions and jsonPatches as these are unrelated patches to /spec/template/spec/kubeadmConfigSpec/users.

- definitions: - jsonPatches: - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs value: [] selector: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate matchResources: controlPlane: true description: Initializes the API server extra sans. name: initializeKubeAPIServerExtraSans - definitions: - jsonPatches: - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs valueFrom: variable: apiServerSigningCertExtraCertSANs selector: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate matchResources: controlPlane: true description: Sets the extraSANs for a cluster enabledIf: '{{ if . apiServerSigningCertExtraCertSANs }}true{{ end }}' name: extraSANs

or simplified (untested):

- definitions: - jsonPatches: - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs value: [] - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs valueFrom: variable: apiServerSigningCertExtraCertSANs selector: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate matchResources: controlPlane: true description: Sets the extraSANs for a cluster enabledIf: '{{ if . apiServerSigningCertExtraCertSANs }}true{{ end }}' name: extraSANs

dkoshkin · 2024-01-27T05:27:04Z

templates/cluster-template-clusterclass.yaml

+  - name: apiServerSigningCertExtraCertSANs
+    required: true
+    schema:
+      openAPIV3Schema:
+        description: Set extra Subject Alternative Names (SANs) for the API Server
+          signing certificate.
+        type: string


And then you can make this an optional list.

Suggested change

- name: apiServerSigningCertExtraCertSANs

required: true

schema:

openAPIV3Schema:

description: Set extra Subject Alternative Names (SANs) for the API Server

signing certificate.

type: string

- name: apiServerSigningCertExtraCertSANs

schema:

openAPIV3Schema:

description: Set extra Subject Alternative Names (SANs) for the API Server

signing certificate.

type: array

items:

type: string

deepakm-ntnx marked this pull request as draft January 24, 2024 23:53

nutanix-cn-prow-bot bot requested review from adiantum and thunderboltsid January 24, 2024 23:53

deepakm-ntnx changed the title ~~Certsan patch~~ [WIP] Certsan patch Jan 25, 2024

deepakm-ntnx changed the title ~~[WIP] Certsan patch~~ CertSAN added as a patch json in clusterclass patches Jan 25, 2024

deepakm-ntnx removed request for adiantum and thunderboltsid January 25, 2024 23:52

deepakm-ntnx changed the title ~~CertSAN added as a patch json in clusterclass patches~~ [WIP] CertSAN added as a patch json in clusterclass patches Jan 25, 2024

deepakm-ntnx added do-not-merge/work-in-progress and removed do-not-merge/work-in-progress labels Jan 25, 2024

deepakm-ntnx changed the title ~~[WIP] CertSAN added as a patch json in clusterclass patches~~ CertSAN added as a patch json in clusterclass patches Jan 26, 2024

deepakm-ntnx marked this pull request as ready for review January 26, 2024 00:17

deepakm-ntnx requested review from jimmidyson, tuxtof, adiantum, thunderboltsid and yannickstruyf3 January 26, 2024 00:17

thunderboltsid reviewed Jan 26, 2024

View reviewed changes

deepakm-ntnx added 5 commits January 26, 2024 11:09

removed enable-hostpath-provisioner entry

5749ce9

reason: it was carry forward from docker provider and is not needed for nutanix provider kcp and kcpt

reverted changed for older version yamls in tests to keep version parity

0e7f589

made certSAN into a patch without variable at this time

a0371ac

removed midflight changes from another PR

702e470

removed midflight changes from another PR

368500f

deepakm-ntnx added 3 commits January 26, 2024 11:10

fixed the map in the patch

3db0b9f

added variable for certSAN

b1717cd

added blackduck scan fix

b6556c9

deepakm-ntnx force-pushed the certsan-patch branch from 095fccd to b6556c9 Compare January 26, 2024 19:11

reverted as BD scan is still failing

26cba45

dkoshkin reviewed Jan 27, 2024

View reviewed changes

deepakm-ntnx mentioned this pull request Mar 22, 2024

feat: Add support for CAPX d2iq-labs/cluster-api-runtime-extensions-nutanix#6

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CertSAN added as a patch json in clusterclass patches #372

CertSAN added as a patch json in clusterclass patches #372

deepakm-ntnx commented Jan 24, 2024 •

edited

Loading

codecov bot commented Jan 24, 2024 •

edited

Loading

thunderboltsid commented Jan 26, 2024

thunderboltsid Jan 26, 2024 •

edited

Loading

deepakm-ntnx Jan 26, 2024

thunderboltsid Jan 26, 2024

deepakm-ntnx Jan 26, 2024

thunderboltsid Jan 26, 2024 •

edited

Loading

thunderboltsid Jan 26, 2024

thunderboltsid Jan 26, 2024

thunderboltsid Jan 26, 2024

dkoshkin Jan 27, 2024 •

edited

Loading

dkoshkin Jan 27, 2024

CertSAN added as a patch json in clusterclass patches #372

Are you sure you want to change the base?

CertSAN added as a patch json in clusterclass patches #372

Conversation

deepakm-ntnx commented Jan 24, 2024 • edited Loading

codecov bot commented Jan 24, 2024 • edited Loading

Codecov Report

thunderboltsid commented Jan 26, 2024

thunderboltsid Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

deepakm-ntnx Jan 26, 2024

Choose a reason for hiding this comment

thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

deepakm-ntnx Jan 26, 2024

Choose a reason for hiding this comment

thunderboltsid Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

dkoshkin Jan 27, 2024 • edited Loading

Choose a reason for hiding this comment

dkoshkin Jan 27, 2024

Choose a reason for hiding this comment

deepakm-ntnx commented Jan 24, 2024 •

edited

Loading

codecov bot commented Jan 24, 2024 •

edited

Loading

thunderboltsid Jan 26, 2024 •

edited

Loading

thunderboltsid Jan 26, 2024 •

edited

Loading

dkoshkin Jan 27, 2024 •

edited

Loading