Support distroless (and distroful) Envoy images #391

nathancoleman · 2022-09-22T17:27:46Z

Changes proposed in this PR:

Remove dependency on shell by:

Creating a Secret containing the Consul CA cert for each gateway deployment
1. Note Creating this secret per-gateway is important because the Secret must reside in the same namespace as the Pod that we will be mounting the volume into, and Gateways are not restricted to a single namespace.
2. Note Creating/updating this Secret requires updating RBAC for the controller over in consul-k8s. I considered using a ConfigMap instead of a Secret - which we already conveniently have permissions to create/update - but consul-k8s uses Secrets for the ca cert and it feels better to respect that precedent.
Mounting a volume on each gateway deployment sourcing the CA cert from the Secret
Configuring an envar on each gateway deployment w/ the location of the CA cert in the mounted volume
Run consul-api-gateway binary directly without the export or exec that we previously depended on

How I've tested this PR:

Deploy consul-k8s with the API gateway controller enabled (apiGateway.enabled: true) and a build of this branch
Modify the consul-api-gateway GatewayClassConfig to use envoyproxy/envoy-distroless instead of envoyproxy/envoy
Create a Gateway and verify that it works correctly

How I expect reviewers to test this PR:

See above

Checklist:

Tests added
CHANGELOG entry added

Run make changelog-entry for guidance in authoring a changelog entry, and
commit the resulting file, which should have a name matching your PR number.
Entries should use imperative present tense (e.g. Add support for...)

Secret is assumed to exist at this point. Future commits will create the secret. Co-Authored-By: sarahalsmiller <[email protected]>

This allows our Gateway image to run on envoy-distroless 🎉 Co-Authored-By: sarahalsmiller <[email protected]>

We create a secret per gateway since the secret must be in the same namespace as the gateway pod in order to reference it in a volume mount

internal/k8s/controllers/gateway_controller.go

.github/workflows/conformance_with_build.yml

This branch will not pass conformance tests until we can pin to consul-k8s v0.49.0, which hasn't been released yet. This is because of a dependency on RBAC changes.

nathancoleman

Personal review

Note Conformance tests won't pass until consul-k8s v0.49.0, which contains the necessary RBAC changes for create/update Secret, is released and consumed

nathancoleman · 2022-09-28T20:24:31Z

config/rbac/role.yaml

@@ -94,8 +94,10 @@ rules:
  resources:
  - secrets
  verbs:
+  - create


The controller needs to be able to create Secrets to hold the Consul CA cert for each gateway Deployment. I used a Secret to be consistent w/ consul-k8s which uses a Secret for the same value.

nathancoleman · 2022-09-28T20:25:45Z

internal/k8s/builder/gateway.go

+					Name:  api.HTTPCAFile,
+					Value: consulCALocalFile,


This adds the path to the CA cert in the pod's environment. This was previously done inside the shell script below as

export CONSUL_CACERT={{ .ConsulCAFile }}

nathancoleman · 2022-09-28T20:26:32Z

internal/k8s/builder/gateway.go

+			Command: []string{"/bootstrap/consul-api-gateway", "exec"},
+			Args:    b.execArgs(),


Instead of running the shell with one giant argument that was our startup script, we're now running consul-api-gateway exec with many small arguments/flags

nathancoleman · 2022-09-28T20:28:17Z

internal/k8s/builder/gateway.go

+				Secret: &corev1.SecretVolumeSource{
+					SecretName: b.gateway.Name,
+					Items: []corev1.KeyToPath{
+						{
+							Key:  "consul-ca-cert",
+							Path: consulCAFilename,
+						},
+					},
+					DefaultMode: nil,
+					Optional:    pointer.Bool(false),
+				},


Instead of creating an empty volume and then having our shell script insert the CA cert, we now source the volume's contents from the Secret containing the CA cert, which is created further down in internal/k8s/reconciler/deployer.go

nathancoleman · 2022-09-28T20:29:53Z

internal/k8s/builder/gateway.go

+// gwContainerArgsTpl is the template for the command arguments executed in the Envoy container.
+// The resulting args are split on \n to obtain a []string for the pod spec's args.
+// Note: Make sure not to leave whitespace at the beginning or end of any line.
+const gwContainerArgsTpl = `


Left the godoc here to explain since the template looks kinda weird now.

Basically, the flag name and its value are expected to be two separate items in the args array for the pod template. It was still easier to compile the list of args using a template like we were before since the other options is a big string builder with a bunch of conditionally-included fmt.Sprintfs.

nathancoleman · 2022-09-28T20:30:52Z

internal/k8s/gatewayclient/gatewayclient.go

@@ -388,15 +389,19 @@ func (g *gatewayClient) Update(ctx context.Context, obj client.Object) error {
 	return nil
 }

-func (g *gatewayClient) CreateOrUpdateDeployment(ctx context.Context, deployment *apps.Deployment, mutators ...func() error) (bool, error) {
-	operation, err := controllerutil.CreateOrUpdate(ctx, g.Client, deployment, func() error {
+func multiMutatorFn(mutators []func() error) func() error {


abstracting this out into a function wrapper since we needed the same logic in one more place now for CreateOrUpdateSecret

nathancoleman · 2022-09-28T20:31:37Z

internal/k8s/reconciler/deployer.go

@@ -52,6 +54,10 @@ func (d *GatewayDeployer) Deploy(ctx context.Context, gateway *K8sGateway) error
 		return err
 	}

+	if err := d.ensureSecret(ctx, gateway.Gateway); err != nil {


You can see here that we create the Secret containing the CA Cert before we create the Deployment that needs to reference the Secret for the contents of one of its volumes.

nathancoleman · 2022-09-28T20:32:17Z

pkg/apis/v1alpha1/types.go

 // MergeService merges a gateway service a onto b and returns b, overriding all of
 // the fields that we'd normally set for a service deployment. It does not attempt
 // to change the service type
 func MergeService(a, b *corev1.Service) *corev1.Service {
 	if !compareServices(a, b) {
-		a.Annotations = b.Annotations
+		b.Annotations = a.Annotations


This seemed like an oversight. Not sure if it's had any material impact on our software or not up until now

nathancoleman · 2022-09-28T20:32:51Z

pkg/apis/v1alpha1/types.go

@@ -177,12 +177,39 @@ type MeshServiceList struct {
 	Items []MeshService `json:"items"`
 }

+func MergeSecret(a, b *corev1.Secret) *corev1.Secret {


Ripped off from the MergeService logic below. Just need to determine what the final state of the Secret should be if we're modifying one that already exists.

.changelog/391.txt

mikemorris · 2022-09-29T17:52:23Z

internal/k8s/builder/testdata/tls-cert.deployment.golden.yaml

-          export CONSUL_CACERT=/consul/tls/ca.pem
-          cat <<EOF >/consul/tls/ca.pem
-          CONSUL_CA_MOCKED
-          EOF


This looks a little different from other tests, should any of this be reflected in the new CA cert logic?

The CONSUL_CACERT envvar is now added to the pod spec below in this case, and the cert content ("CONSUL_CA_MOCKED" here) would now be deployed into a secret that the volume mount below references.

I did refine the code a bit just now to only include the envvar and create the secret when the CA is actually required, so now it's clearer that the resulting deployment for this test case which does require the CA is different from the others which don't require the CA.

mikemorris

Holding approval until conformance tests are updated to consul-k8s v0.49.0 and passing (added pr/conformance label so it's clear this is currently blocked on failing tests), but approach LGTM.

mikemorris · 2022-09-29T19:01:23Z

Should this update or add a CI test with distroless images as #152 did?

nathancoleman · 2022-09-29T20:47:06Z

@mikemorris that gets difficult because consul-k8s doesn't support distroless yet, so we can't just set imageEnvoy in our values.yaml file. I figured it'd be easier to wait until they do and then add to our matrix. Thoughts?

Mount CA cert from secret into gateway pod

ec2746a

Secret is assumed to exist at this point. Future commits will create the secret. Co-Authored-By: sarahalsmiller <[email protected]>

nathancoleman assigned nathancoleman and sarahalsmiller Sep 22, 2022

Remove shell exec, run consul-api-gateway directly

2975768

This allows our Gateway image to run on envoy-distroless 🎉 Co-Authored-By: sarahalsmiller <[email protected]>

nathancoleman force-pushed the support-distroless branch from 885a2e2 to 2975768 Compare September 22, 2022 18:11

nathancoleman added 2 commits September 22, 2022 15:43

Create secret per gateway containing CA cert

ab33d5b

We create a secret per gateway since the secret must be in the same namespace as the gateway pod in order to reference it in a volume mount

Upsert gateway secret instead of just checking for existence

41ca303

nathancoleman changed the title ~~Mount CA cert from secret into gateway pod~~ Support distroless Envoy images Sep 22, 2022

nathancoleman added 5 commits September 22, 2022 16:42

Regenerate mock gatewayclient

170fd0a

Add/update unit test coverage

8da63e1

Add RBAC entry for create + update of secrets

5a850af

Add changelog entry

537189b

Regenerate golden files for deployment assertions

12460aa

nathancoleman mentioned this pull request Sep 22, 2022

Upgrade default envoy image to use 1.22 distroless #152

Closed

Update kubebuilder annotation to include additional verbs for secrets

581cff7

nathancoleman force-pushed the support-distroless branch from 78b6a56 to 581cff7 Compare September 22, 2022 21:19

Use dev branch of consul-k8s containing RBAC changes for conformance

f4ef203

nathancoleman added the do not merge label Sep 22, 2022

nathancoleman added 2 commits September 23, 2022 13:17

Remove now-unnecessary quotes from gateway-host value

ccb5354

Add Secret to list of resources owned by Gateway

bf5d64c

nathancoleman commented Sep 23, 2022

View reviewed changes

internal/k8s/controllers/gateway_controller.go Show resolved Hide resolved

Regenerate golden files for deployment assertions

cf49e03

nathancoleman added the pr/run-conformance label Sep 23, 2022

nathancoleman mentioned this pull request Sep 27, 2022

Allow the API Gateway controller to create and update Secrets hashicorp/consul-k8s#1542

Merged

2 tasks

hc-github-team-consul-ecosystem mentioned this pull request Sep 28, 2022

Backport of Allow the API Gateway controller to create and update Secrets into release/0.49.x hashicorp/consul-k8s#1546

Merged

2 tasks

Write mutated Secret w/ owner reference so it's cleaned up automatically

46f6a63

nathancoleman added pr/conformance Run conformance tests from kubernetes-sigs/gateway-api and removed do not merge pr/run-conformance labels Sep 28, 2022

nathancoleman marked this pull request as ready for review September 28, 2022 19:50

nathancoleman commented Sep 28, 2022

View reviewed changes

.github/workflows/conformance_with_build.yml Outdated Show resolved Hide resolved

nathancoleman removed the pr/conformance Run conformance tests from kubernetes-sigs/gateway-api label Sep 28, 2022

nathancoleman force-pushed the support-distroless branch from 55ba961 to 46f6a63 Compare September 28, 2022 20:14

nathancoleman requested review from sarahalsmiller, andrewstucki and mikemorris September 28, 2022 20:15

nathancoleman added 3 commits September 28, 2022 16:16

Restore consul-k8s pin for conformance tests

ac53d05

This branch will not pass conformance tests until we can pin to consul-k8s v0.49.0, which hasn't been released yet. This is because of a dependency on RBAC changes.

Merge branch 'main' into support-distroless

e6e6dac

Replace outdated comment with godoc

f4f42ea

nathancoleman commented Sep 28, 2022

View reviewed changes

nathancoleman changed the title ~~Support distroless Envoy images~~ Support distroless (and distroful) Envoy images Sep 28, 2022

nathancoleman commented Sep 28, 2022

View reviewed changes

.changelog/391.txt Outdated Show resolved Hide resolved

Update .changelog/391.txt

77f71c9

mikemorris reviewed Sep 29, 2022

View reviewed changes

mikemorris added the pr/conformance Run conformance tests from kubernetes-sigs/gateway-api label Sep 29, 2022

mikemorris reviewed Sep 29, 2022

View reviewed changes

nathancoleman added 2 commits September 30, 2022 11:38

Consume consul-k8s v0.49.0 for conformance testing

9bed1da

Only add CONSUL_CACERT to pod env when required

9df8349

nathancoleman force-pushed the support-distroless branch from 096dcd4 to 9df8349 Compare September 30, 2022 19:46

Only create Secret if gateway config requires CA

871ef41

nathancoleman requested a review from mikemorris September 30, 2022 20:12

mikemorris approved these changes Oct 3, 2022

View reviewed changes

Merge branch 'main' into support-distroless

a96eaf9

nathancoleman merged commit 5e992cd into main Oct 3, 2022

nathancoleman deleted the support-distroless branch October 3, 2022 17:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support distroless (and distroful) Envoy images #391

Support distroless (and distroful) Envoy images #391

nathancoleman commented Sep 22, 2022 •

edited

Loading

nathancoleman left a comment

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

nathancoleman Sep 28, 2022

mikemorris Sep 29, 2022

nathancoleman Sep 30, 2022

mikemorris left a comment •

edited

Loading

mikemorris commented Sep 29, 2022

nathancoleman commented Sep 29, 2022

		Command: []string{"/bootstrap/consul-api-gateway", "exec"},
		Args: b.execArgs(),

Support distroless (and distroful) Envoy images #391

Support distroless (and distroful) Envoy images #391

Conversation

nathancoleman commented Sep 22, 2022 • edited Loading

Changes proposed in this PR:

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

nathancoleman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikemorris left a comment • edited Loading

Choose a reason for hiding this comment

mikemorris commented Sep 29, 2022

nathancoleman commented Sep 29, 2022

nathancoleman commented Sep 22, 2022 •

edited

Loading

mikemorris left a comment •

edited

Loading