USHIFT-2455 USHIFT-2456 USHIFT-2457 USHIFT-2458: Introduce adding Custom CA certs for API endpoint #3130

eslutsky · 2024-03-10T13:39:22Z

No description provided.

openshift-ci-robot · 2024-03-10T13:39:25Z

@eslutsky: This pull request references USHIFT-2458 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.16.0" version, but no target version was set.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-03-10T13:39:31Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci-robot · 2024-03-13T15:03:28Z

@eslutsky: This pull request references USHIFT-2101 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.16.0" version, but no target version was set.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

eslutsky · 2024-03-13T15:35:35Z

/test all

eslutsky · 2024-03-14T15:11:34Z

/test all

eslutsky · 2024-03-27T15:44:24Z

/test all

eslutsky · 2024-03-27T18:13:38Z

/test all

pmtk

Just couple, rather minor, comments, but maybe someone more knowledgeable about certs could take a look

pmtk · 2024-04-02T12:33:35Z

pkg/controllers/kube-apiserver.go

+				},
+			}
+			// prepend the named certs to the beginning of the slice (so it will take precedence for same SNI)
+			namedCerts = append(cert, namedCerts...)


Okay, so that works functionally.

Just FYI: This is very inefficient. Go will check capacity of cert and since it's 1, it will allocate new slice. This will happen for all iterations of the loop. We don't expect huge amounts of data in the slice so it's "alright"

pkg/cmd/init.go

pkg/util/cert.go

pkg/util/net.go

test/resources/openssl.resource

pkg/util/cert.go

pkg/cmd/init.go

cmd/generate-config/config/config-openapi-spec.json

packaging/microshift/config.yaml

pkg/cmd/init.go

test/suites/standard2/validate-custom-certificates.robot

Signed-off-by: Evgeny Slutsky <[email protected]>

openshift-ci-robot · 2024-04-09T09:02:38Z

@eslutsky: This pull request references USHIFT-2458 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.16.0" version, but no target version was set.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

pmtk · 2024-04-11T08:07:44Z

pkg/cmd/init.go

@@ -408,6 +408,60 @@ func initKubeconfigs(
 		klog.Warningf("Unable to remove stale kubeconfigs: %v", err)


Just realized, do we cleanup stale kubeconfigs for NamedCertificates no longer present in the config?

yes its taken cared before generating the kubeconfig by running cleanupStaleKubeconfigs, https://github.com/openshift/microshift/blob/main/pkg/cmd/init.go#L407

test/assets/custom-certs/csr-no-san.conf.template

pmtk

Two minor things, otherwise it looks pretty good

Signed-off-by: Evgeny Slutsky <[email protected]>

pmtk · 2024-04-11T13:55:28Z

/lgtm

openshift-ci · 2024-04-11T14:03:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eslutsky, pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [eslutsky,pmtk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2024-04-11T14:41:26Z

@eslutsky: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 10, 2024

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 10, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 10, 2024

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch 2 times, most recently from f93b3fe to 0871763 Compare March 13, 2024 08:55

eslutsky mentioned this pull request Mar 13, 2024

USHIFT-2166: introduce Custom CAs POC #2892

Closed

eslutsky changed the title ~~USHIFT-2458: add custom certificate e2e tests~~ USHIFT-2458: introduce custom certificates Mar 13, 2024

This was referenced Mar 13, 2024

USHIFT-2455: introduce NamedCertificate config #3077

Closed

USHIFT-2456: add custom certs to kube apiserver #3078

Closed

eslutsky changed the title ~~USHIFT-2458: introduce custom certificates~~ USHIFT-2101: Introduce adding Custom CA certs for API endpoint Mar 13, 2024

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from 0871763 to 4efac90 Compare March 13, 2024 15:34

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch 3 times, most recently from 1dc3727 to d3208bc Compare March 14, 2024 14:06

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from d3208bc to fb4ee76 Compare March 27, 2024 15:35

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from fb4ee76 to 0aa849a Compare March 27, 2024 15:48

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from 0aa849a to c09851e Compare March 28, 2024 10:36

eslutsky marked this pull request as ready for review March 28, 2024 10:37

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 28, 2024

eslutsky changed the title ~~USHIFT-2101: Introduce adding Custom CA certs for API endpoint~~ WIP: USHIFT-2101: Introduce adding Custom CA certs for API endpoint Mar 28, 2024

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 28, 2024

openshift-ci bot requested review from dhellmann and pliurh March 28, 2024 10:38

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch 4 times, most recently from 320ece3 to d20d640 Compare April 2, 2024 11:30

pmtk reviewed Apr 2, 2024

View reviewed changes

pacevedom reviewed Apr 2, 2024

View reviewed changes

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from d20d640 to b511e24 Compare April 4, 2024 12:39

ggiguash suggested changes Apr 7, 2024

View reviewed changes

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from b511e24 to a1fc792 Compare April 8, 2024 09:58

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2024

eslutsky added 3 commits April 8, 2024 15:39

USHIFT-2455: introduce NamedCertificate config

6c9d969

Signed-off-by: Evgeny Slutsky <[email protected]>

USHIFT-2456: add namecertificates to kube-apiserver

9989738

Signed-off-by: Evgeny Slutsky <[email protected]>

USHIFT-2457: Generate kubeconfigs for custom certs

257b1ec

Signed-off-by: Evgeny Slutsky <[email protected]>

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from a1fc792 to fe5539b Compare April 8, 2024 12:39

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2024

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from fe5539b to a40e8fb Compare April 8, 2024 13:14

eslutsky requested a review from pacevedom April 9, 2024 07:45

eslutsky changed the title ~~USHIFT-2101: Introduce adding Custom CA certs for API endpoint~~ USHIFT-2455 USHIFT-2456 USHIFT-2457 USHIFT-2458: Introduce adding Custom CA certs for API endpoint Apr 9, 2024

pmtk reviewed Apr 11, 2024

View reviewed changes

test/assets/custom-certs/csr-no-san.conf.template Outdated Show resolved Hide resolved

pmtk reviewed Apr 11, 2024

View reviewed changes

USHIFT-2458: add custom certificates RF tests

3acbfc1

Signed-off-by: Evgeny Slutsky <[email protected]>

eslutsky force-pushed the USHIFT-2458-Certs-RF-Tests branch from a40e8fb to 3acbfc1 Compare April 11, 2024 13:08

openshift-ci bot assigned pmtk Apr 11, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2024

openshift-merge-bot bot merged commit a372f80 into openshift:main Apr 11, 2024

		@@ -408,6 +408,60 @@ func initKubeconfigs(
		klog.Warningf("Unable to remove stale kubeconfigs: %v", err)

USHIFT-2455 USHIFT-2456 USHIFT-2457 USHIFT-2458: Introduce adding Custom CA certs for API endpoint #3130

USHIFT-2455 USHIFT-2456 USHIFT-2457 USHIFT-2458: Introduce adding Custom CA certs for API endpoint #3130

Uh oh!

Conversation

eslutsky commented Mar 10, 2024

Uh oh!

openshift-ci-robot commented Mar 10, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Mar 10, 2024

Uh oh!

openshift-ci-robot commented Mar 13, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eslutsky commented Mar 13, 2024

Uh oh!

eslutsky commented Mar 14, 2024

Uh oh!

eslutsky commented Mar 27, 2024

Uh oh!

eslutsky commented Mar 27, 2024

Uh oh!

pmtk left a comment

Choose a reason for hiding this comment

Uh oh!

pmtk Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 9, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pmtk Apr 11, 2024

Choose a reason for hiding this comment

Uh oh!

eslutsky Apr 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pmtk left a comment

Choose a reason for hiding this comment

Uh oh!

pmtk commented Apr 11, 2024

Uh oh!

openshift-ci bot commented Apr 11, 2024

Uh oh!

openshift-ci bot commented Apr 11, 2024

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 10, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Mar 13, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 9, 2024 •

edited by openshift-ci bot

Loading