feat(spin/certs): automate creating the default CA bundle secret by adamreese · Pull Request #207 · spinframework/spin-operator

adamreese · 2024-04-17T00:39:04Z

Supersedes #184

Automate the creation of a secret for a default CA root certificate bundle. A secret is created in each namespace that contains a spin application. If a secret already exists with the name spin-ca it will not be modified. This allows the default spin-ca secret to be overridden by the user.

The embedded CA bundle is fetched from https://curl.se/ca/cacert.pem and can be updated to the latest by running go generate ./....

There is no owner reference on the secret which means it will persist unless manually deleted. Meaning that if spin-operator is removed from the cluster it will not be included in the cascading deletion.

This commit injects a default CA bundle in the root filesystem for every Spin application pod created, so host components can execute TLS operations. Signed-off-by: Radu Matei <radu@fermyon.com>

calebschoepp

Great work Adam.

There is no owner reference on the secret which means it will persist unless manually deleted. Meaning that if spin-operator is removed from the cluster it will not be included in the cascading deletion.

Mind elaborating on this design choice. My gut feeling is that we would want it do be cleaned up automatically.

internal/controller/spinapp_controller.go

internal/cacerts/cacerts.go

adamreese · 2024-04-17T18:19:58Z

@calebschoepp Having no owner reference is because there isn't anything to assign ownership to. The secret is global for the namespace so it can't be owned by specific applications.

calebschoepp · 2024-04-17T20:32:45Z

@calebschoepp Having no owner reference is because there isn't anything to assign ownership to. The secret is global for the namespace so it can't be owned by specific applications.

Could it be owned by the operator so that if the operator is removed the certs are removed too?

adamreese · 2024-04-17T21:39:16Z

The operator isn't aware of it's own kubernetes deployment. It's just a running binary.

endocrimes · 2024-04-17T21:51:41Z

internal/cacerts/cacerts.go

+// Package cacerts provides an embedded CA root certificates bundle.
+package cacerts
+
+//go:generate curl -sfL https://curl.se/ca/cacert.pem -o ca-certificates.crt


LazyQ: What license are these under? if it involves attribution are we correctly doing so?

Great question. It's built off the mozilla cert bundle which is under Mozilla Public License

I think this means the cacerts package also has to be MPL-2.0 (because we embed the MPL-2.0 data in the sources), and MPL-2.0 is (albeit weakly) copyleft.

I'm not exactly sure what we need to do for this one.

I think a comment on the variable that the contents are MPL-2.0 licensed might be good enough? (and having a link to the cert bundle is helpful for understanding what they are anyway)

internal/controller/spinapp_controller.go

michelleN

+1 to adding fields to the SpinAppExecutor around defaulting behavior and secret reference for CA bundle

bacongobbler · 2024-05-06T15:18:20Z

RE: #207 (comment)

Disclaimer: I am not a lawyer.

I think this covers the section under the MPL-2.0 terms for "Distribution of Source Form" (Section 3.1 of the MPL-2.0), but it does not cover "Distribution of Executable Form".

Section 3.2 of the MPL 2.0 states the following:

If You distribute Covered Software in Executable Form then:

a. such Covered Software must also be made available in Source Code Form, as described in Section 3.1, and You must inform recipients of the Executable Form how they can obtain a copy of such Source Code Form by reasonable means in a timely manner, at a charge no more than the cost of distribution to the recipient; and

b. You may distribute such Executable Form under the terms of this License, or sublicense it under different terms, provided that the license for the Executable Form does not attempt to limit or alter the recipients’ rights in the Source Code Form under this License.

Given that our users would refer to the LICENSE file provided with the executable, I think we need to include it there as well. Either that or include a NOTICE file referring to the MPL-2.0 license and its usage within spin-operator.

bacongobbler · 2024-05-06T15:22:46Z

Hmm.. Re-reading that section, it doesn't state you need to sub-license the project or include its terms in a NOTICE file. You just need to make the "recipient of the Executable Form" aware of how to receive the source code.

I think we're good on that front... But again, not a lawyer. :)

bacongobbler

Looks good to me! Left a few quick comments but I think this is safe to merge at this point.

bacongobbler · 2024-05-06T15:23:55Z

internal/cacerts/cacerts.go

+//
+//   curl -sfL https://curl.se/ca/cacert.pem -o ca-certificates.crt
+
+import _ "embed"


TIL this is a part of the standard library now. That's awesome

bacongobbler · 2024-05-06T15:28:31Z

internal/controller/deployment.go

 // exclusive with a user-provided secret - this is to require _either_ a
 // manual runtime-config or a generated one from the crd.
-func ConstructVolumeMountsForApp(ctx context.Context, app *spinv1alpha1.SpinApp, generatedRuntimeSecret string) ([]corev1.Volume, []corev1.VolumeMount, error) {
+func ConstructVolumeMountsForApp(ctx context.Context, app *spinv1alpha1.SpinApp, generatedRuntimeSecret, caSecretName string) ([]corev1.Volume, []corev1.VolumeMount, error) {


I wish there was a way to make caSecretName use an Option<string> instead of an empty string to make it more clear to the caller that this is an optional value. I've been spoiled by Rust.

we can always use *string in golang. (before you ask Even I am not sure if I am kidding or being serious here.)

internal/controller/spinapp_controller.go

Supersedes spinframework#184 Automate the creation of a secret for a default CA root certificate bundle. A secret is created in each namespace that contains a spin application. If a secret already exists with the name `spin-ca` it will not be modified. This allows the default `spin-ca` secret to be overridden by the user. The embedded CA bundle is fetched from https://curl.se/ca/cacert.pem and can be updated to the latest by running `go generate ./...`. There is no owner reference on the secret which means it will persist unless manually deleted. Meaning that if spin-operator is removed from the cluster it will not be included in the cascading deletion. Signed-off-by: Adam Reese <adam@reese.io>

fix(spin/certs): inject CA bunde in the filesystem

679c6f3

This commit injects a default CA bundle in the root filesystem for every Spin application pod created, so host components can execute TLS operations. Signed-off-by: Radu Matei <radu@fermyon.com>

adamreese requested review from bacongobbler, calebschoepp, endocrimes and michelleN as code owners April 17, 2024 00:39

calebschoepp reviewed Apr 17, 2024

View reviewed changes

internal/controller/spinapp_controller.go Show resolved Hide resolved

internal/controller/spinapp_controller.go Outdated Show resolved Hide resolved

internal/cacerts/cacerts.go Outdated Show resolved Hide resolved

endocrimes reviewed Apr 17, 2024

View reviewed changes

internal/controller/spinapp_controller.go Outdated Show resolved Hide resolved

michelleN reviewed Apr 17, 2024

View reviewed changes

radu-matei mentioned this pull request Apr 22, 2024

fix(spin/certs): inject CA bunde in the filesystem #184

Closed

adamreese force-pushed the feat/inject-ca branch 2 times, most recently from a80b180 to 3d2a27f Compare April 22, 2024 19:54

radu-matei mentioned this pull request Apr 23, 2024

unable to hit external API from SPIN Go component running in SpinKube #216

Closed

adamreese force-pushed the feat/inject-ca branch 2 times, most recently from b5f092e to e69223b Compare April 24, 2024 21:06

adamreese force-pushed the feat/inject-ca branch from e69223b to ad32617 Compare May 1, 2024 23:43

endocrimes approved these changes May 6, 2024

View reviewed changes

bacongobbler approved these changes May 6, 2024

View reviewed changes

adamreese force-pushed the feat/inject-ca branch from ad32617 to 8bca9ca Compare May 9, 2024 22:12

michelleN merged commit 7e8d78e into spinframework:main May 15, 2024

adamreese deleted the feat/inject-ca branch May 15, 2024 15:06

endocrimes mentioned this pull request May 21, 2024

spinkube: bump to 0.2.0 civo/kubernetes-marketplace#780

Merged

4 tasks

rajatjindal mentioned this pull request May 23, 2024

HTTPS request fails spinframework/containerd-shim-spin#44

Closed

Conversation

adamreese commented Apr 17, 2024

Uh oh!

calebschoepp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamreese commented Apr 17, 2024

Uh oh!

calebschoepp commented Apr 17, 2024

Uh oh!

adamreese commented Apr 17, 2024

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!

Uh oh!

michelleN left a comment

Choose a reason for hiding this comment

Uh oh!

bacongobbler commented May 6, 2024

Uh oh!

bacongobbler commented May 6, 2024

Uh oh!

bacongobbler left a comment

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants