New by esposem · Pull Request #24 · confidential-devhub/cococtl

esposem · 2026-02-16T16:24:58Z

init: move runtimeclass setup in a separate function: just refactoring
apply: get rid of getTrusteeNamespace in favor of cfg.GetTrusteeNamespace: looks like they are the same function
init: skip CA cert upload if the Trustee URL is not defined, just generate the files: I believe this was a bug, if we don't want to deploy we shouldn't upload
Use right default Trustee namespace: default to "trustee-operator-system" instead of "default"
init/apply: add certDir to customize where certificates are stored/loaded when needed by sidecar: custom location useful especially to default existing certs to be loaded later by the apply command.
apply: introduce cert-dir that overrides the --cert-dir of init: this is used in apply if the user wants to use another location to load (and store) the certs folder
apply: add enableinitdata and trusteeurl, to add the initdata annotation: allow not to add any initdata, and override trustee-url when doing apply
init: trustee-url is now optional, only needed for initdata
init: rename skip-trustee-deploy with trustee-deploy. Same effect but flag is defaulted to the opposite
apply: rename skip-apply with apply. Same effect but flag is defaulted to the opposite
apply: change the sidecar certs URI to be in the kbs://default Trustee ns: fix this as Trustee operator defaults with "default" ns.
fix various tests failling because of this change

cmd/init.go

esposem · 2026-02-17T14:53:29Z

@bpradipt please take a look now. deploy and apply are still there, just inverted. Rest are just improvements here and there

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

…pace

…erate the files Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

…aded when needed by sidecar Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

… flag is defaulted to the opposite

…d to the opposite

…e ns Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

bpradipt · 2026-02-27T10:03:41Z

cmd/init.go

+
+	fmt.Println()
+	fmt.Printf("Using provided RuntimeClass: %s\n", cfg.RuntimeClass)
+}


this makes sense
/lgtm

P.S. I'm reviewing individual commits

bpradipt · 2026-02-27T10:04:40Z

cmd/apply.go

-			} else {
-				fmt.Printf("  ✓ Successfully added %d imagePullSecret(s) to Trustee\n", len(imagePullSecretRefs))
-			}
+			fmt.Printf("  ✓ Successfully added %d imagePullSecret(s) to Trustee\n", len(imagePullSecretRefs))


bpradipt · 2026-02-27T10:09:31Z

cmd/init.go

 		sidecarNamespace = trusteeNamespace
 		if sidecarNamespace == "" {
-			sidecarNamespace = "default"
+			sidecarNamespace = config.DefaultTrusteeNamespace


Let's park this commit on default Trustee namespace for now. Need to think few things around this.

bpradipt · 2026-02-27T10:09:42Z

pkg/config/config_test.go

 		{
-			name:          "external URL - should default to default",
-			trusteeServer: "https://trustee.example.com:8080",
+			name:          "external URL - should default to trustee-operator-system",


Let's park this commit on default Trustee namespace for now. Need to think few things around this.

bpradipt · 2026-02-27T10:12:58Z

pkg/config/config.go

+	if cfg.Sidecar.CertDir == "" {
+		cfg.Sidecar.CertDir, _ = GetDefaultCertDir()
+	}
 	if cfg.Sidecar.Image == "" {


/lgtm.
Having a certDir option to store keys is good. It can be populated out-of-band as well

bpradipt · 2026-02-27T10:13:41Z

cmd/apply.go

 	applyCmd.Flags().BoolVar(&sidecarSkipAutoSANs, "sidecar-skip-auto-sans", false, "Skip auto-detection of SANs (node IPs and service DNS)")
 	applyCmd.Flags().IntVar(&sidecarPortForward, "sidecar-port-forward", 0, "Port to forward from primary container (requires --sidecar)")
 	applyCmd.Flags().StringVarP(&namespaceFlag, "namespace", "n", "", "Namespace for operations (overrides manifest and kubeconfig)")
+	applyCmd.Flags().StringVar(&overrideCertDir, "cert-dir", "", "Override the cert directory used to load/store sidecar certificates and keys")


@esposem I didn't understand the need for this commit ?

bpradipt · 2026-02-27T10:23:46Z

cmd/apply.go

 	configPath          string
 	convertSecrets      bool
 	enableSidecar       bool
+	enableInitData      bool


I understand enableInitData is a toggle to decide whether to add initData during apply. I would say just have this toggle in this commit with a short description in the commit message on the reasoning.
Remove trusteeURL config from this commit. Let's think through the trusteeURL a bit more. Why it's needed for apply and what workflow are we trying to achieve.
Too many options to override is a recipe for both end-user confusion and maintenance headache

bpradipt · 2026-02-27T10:26:44Z

cmd/apply.go


 		fmt.Println("  - Injecting secure access sidecar container")
-		if err := sidecar.Inject(m, cfg, appName, namespace); err != nil {
+		// default the namespace to "default" as Trustee operator currently only supports that


Let's drop this commit for now. The client tool must not be dictated by the capabilities of specific Trustee deployment option.
Also this is not entirely true. You can check with vault for example.

bpradipt · 2026-02-27T10:32:28Z

cmd/init.go

-	if err := trustee.UploadResource(ctx, clientset, trusteeNamespace, clientCAPath, clientCA.CertPEM); err != nil {
-		return fmt.Errorf("failed to upload client CA to KBS: %w", err)
+	var clientCAPath string
+	if cfg.TrusteeServer != "" {


Wondering if cert upload should be dictated by TrusteeURL or an explicit bool.
Personally having an explicit bool during init is clear and self-explanatory.

esposem force-pushed the new branch from d2e0211 to c2ea23c Compare February 17, 2026 09:01

bpradipt reviewed Feb 17, 2026

View reviewed changes

cmd/init.go Show resolved Hide resolved

bpradipt reviewed Feb 17, 2026

View reviewed changes

cmd/init.go Show resolved Hide resolved

esposem force-pushed the new branch 2 times, most recently from a35009d to 2c80477 Compare February 17, 2026 14:52

esposem force-pushed the new branch 2 times, most recently from 68a5d51 to 238c60d Compare February 17, 2026 15:25

This was referenced Feb 18, 2026

New improvements #23

Closed

Improvements #20

Closed

esposem force-pushed the new branch from 238c60d to 5840348 Compare February 19, 2026 12:24

esposem added 18 commits February 19, 2026 13:45

init: move runtimeclass setup in a separate function

ee6cc26

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

apply: get rid of getTrusteeNamespace in favor of cfg.GetTrusteeNames…

a463e88

…pace

init: skip CA cert upload if the Trustee URL is not defined, just gen…

8125a42

…erate the files Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

Use right default Trustee namespace

02e5854

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

fix tests for default Trustee namespace

e695d2a

init/apply: add certDir to customize where certificates are stored/lo…

22546a5

…aded when needed by sidecar Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

add a test for certdir

89c8d08

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

apply: introduce cert-dir that overrides the --cert-dir of init

f5ddbcf

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

add a test for override certdir

e922e3f

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

apply: add enableinitdata and trusteeurl, to add the initdata annotation

81f8f37

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

fix tests with initdata.Generate()

46818d8

init: trustee-url is now optional, only needed for initdata

9a29f88

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

fix test missing trustee server

9176659

init: rename skip-trustee-deploy with trustee-deploy. Same effect but…

1131c7e

… flag is defaulted to the opposite

fix tests for trustee-deploy

97d14f4

apply: rename skip-apply with apply. Same effect but flag is defaulte…

592efce

…d to the opposite

fix tests for apply

295f200

apply: change the sidecar certs URI to be in the kbs://default Truste…

40e7a29

…e ns Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

esposem force-pushed the new branch from 5840348 to 40e7a29 Compare February 19, 2026 12:58

This was referenced Feb 19, 2026

Improve initdata #25

Open

Add creation of client.p12 when sidecar certs are created in init #26

Open

bpradipt reviewed Feb 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New#24

New#24
esposem wants to merge 18 commits intoconfidential-devhub:mainfrom
esposem:new

esposem commented Feb 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

esposem commented Feb 17, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

bpradipt Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

esposem commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

esposem commented Feb 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

esposem commented Feb 16, 2026 •

edited

Loading