Switch to use fileca in e2e tests #309

jdolitsky · 2022-01-04T16:44:56Z

Resolves #301

jdolitsky · 2022-01-04T16:45:04Z

@mattmoor @vaikas - is it more involved than this? Need to point to a CA file?

mattmoor · 2022-01-04T16:48:24Z

cc @nsmith5

Yeah, we need to set up the secret, mount it, and pass the flag to Fulcio.

nsmith5 · 2022-01-04T16:58:07Z

🚀 Nice!

nsmith5 · 2022-01-04T17:28:15Z

.github/workflows/verify-k8s.yml

-          # Switch to the ephemeralca for testing.
-          sed -i -e 's,--ca=googleca,--ca=ephemeralca,g' ${{ github.workspace }}/config/deployment.yaml
+          # Switch to fileca for testing.
+          sed -i -e 's,--ca=googleca,--ca=fileca,g' ${{ github.workspace }}/config/deployment.yaml


Once the secret is set up and mounted, the fileca needs a couple more flags to just figure out where the certs are mounted and the password to decrypt the key. They're as follows

--fileca-cert for the cert path

--fileca-key for the key path

--fileca-passwd for the key password

--fileca-watch=true/false if you want to watch the cert and key path for updates and reload on change

I'm less familiar with how the deployment is configured, but these flags will need to be set somewhere to get fileca working

jdolitsky · 2022-01-05T00:29:20Z

Current short-term plan here (please let me know if any objections):

Add a simple main.go for generating a new CA cert+key (based on ephemeralca logic)
Run main.go mentioned in previous step to generate a temporary CA cert+key
Create new fulcio-secret.yaml in config/ on the fly, adding the contents of CA cert+key
Modify config/deployment.yaml to mount new secret (optionally)
Fix sed to pass in --fileca-cert and --fileca-key
Extra: Remove sed for replica count, keep 3 replicas (since distributed should work properly now using fileca)

nsmith5 · 2022-01-05T01:16:26Z

Maybe we could skip 1. and just use openssl to generate the key pair? Would save us from maintaining that extra bit of go code. There are lots of examples of generating key-pairs using openssl in pkg/ca/fileca/testdata/generate.sh. E.g

password=password123
duration=36500 # 100 years

# ed25519
openssl req -x509 \
    -newkey ed25519 \
    -sha256 \
    -keyout ed25519-key.pem \
    -out ed25519-cert.pem \
    -subj "/CN=ed25519" \
    -days $duration \
    -addext basicConstraints=critical,CA:TRUE,pathlen:1 \
    -passout pass:"$password"

jdolitsky · 2022-01-05T01:52:15Z

@nsmith5 that sounds good, thanks for sharing paths to example scripts

Resolves sigstore#301 Signed-off-by: Josh Dolitsky <josh@dolit.ski>

jdolitsky · 2022-01-05T16:46:59Z

Regarding my comment "Remove sed for replica count, keep 3 replicas", going to skip this for now as it appears this was done for prometheus vs. CA-related

mattmoor · 2022-01-05T16:58:15Z

.github/workflows/verify-k8s.yml

+          data:
+            key.pem: $(cat "${{ github.run_id }}-key.pem" | base64)
+            cert.pem: $(cat "${{ github.run_id }}-cert.pem" | base64)


FYI you can use stringData and drop the base64 (it's k8s magic ✨ )

hm. is that going to work alright with a multiline file?

It would, but something something yaml 😞

.github/workflows/verify-k8s.yml

jdolitsky · 2022-01-05T23:39:09Z

Pulled in changes from #314 and tests passing now

mattmoor · 2022-01-05T23:49:23Z

config/deployment.yaml

+        - name: fulcio-secret
+          mountPath: /etc/fulcio-secret
+          readOnly: true


I believe this will cause production to break, but I believe you can make it optional (and then it won't)

Does the optional field added in volumes section below accomplish this?

I think so. Tried adding optional in another branch and got yelled at:

error: error validating "STDIN": error validating data: ValidationError(Deployment.spec.template.spec.containers[0].volumeMounts[1]): unknown field "optional" in

.github/workflows/verify-k8s.yml

Signed-off-by: Josh Dolitsky <josh@dolit.ski>

jdolitsky · 2022-01-06T15:47:07Z

config/deployment.yaml

+      - name: fulcio-secret
+        secret:
+          secretName: fulcio-secret
+          optional: true


^^ @mattmoor

That should make it so that it doesn't block starting it. The one interesting bit is what happens if you do want to specify the secret and it is not there yet but you'd like it to actually block. IIRC though, we watch that mount point so I think it will pick it up once it becomes available. @nsmith5 check me out :)

Looks like --fileca-watch flag defaults to true, so I think thats correct @vaikas

fileca will never start without a valid key-pair. We never want to boot up the server if we can't serve requests so I wrote it to try and load an initial key-pair and bail hard if the doesn't work (ref https://github.com/sigstore/fulcio/blob/main/pkg/ca/fileca/fileca.go#L48)

I don't think I know what Kubernetes would do in this case. My hope would be a pod crashloop until the secret mounts, but I don't know the optional spec well enough to know that is the case. Once the pod from this deployment is scheduled and the secret wasn't available at schedule time, does that mean it will never get that secret volume mounted by pods scheduled later when the secret exists will get it? Or does it mean that Kubernetes will try to mount it even to an existing pod once the secret does exist?

The watch behaviour is also pretty paranoid about putting the server in a state where it can't serve requests so it looks for updates, but if, for instance, the cert doesn't match the key or the key can't be decrypted with the password it will just keep using the existing certs and ignore the updates from the filesystem

Ran Matt's suggestion, and things appear to work as we expect:

Create a Pod with an optional secret volume and an image that crash loops if the secret isn't there:

cat << EOF | kubectl apply -f - apiVersion: apps/v1 kind: Deployment metadata: name: boop-deployment labels: app: boop spec: replicas: 1 selector: matchLabels: app: boop template: metadata: labels: app: boop spec: containers: - name: boop image: bash command: - "bash" - "-c" - "[[ -f /boop/bop.txt ]] || (echo bad && exit 1) && (echo good && while true; do sleep 1; done)" volumeMounts: - name: boop-secret mountPath: /boop readOnly: true volumes: - name: boop-secret secret: secretName: boop-secret optional: true EOF

$ k get pods NAME READY STATUS RESTARTS AGE boop-deployment-5c48fd856b-tv9lz 0/1 CrashLoopBackOff 2 33s

Create the secret and see if it stops crash looping.

cat <<EOF | kubectl apply -f - apiVersion: v1 kind: Secret metadata: name: boop-secret data: bop.txt: $(echo boop | base64) EOF

$ k get pods NAME READY STATUS RESTARTS AGE boop-deployment-5c48fd856b-tv9lz 1/1 Running 3 49s

yay?

@vaikas @nsmith5 Did you have anything else on this before we merge?

Nope looks good from me! Good work @jdolitsky :D

Thanks for checking in with Dr. Empirical 😆

No. I think it might behoove us to file an issue in k8s to document the behaviour in case they reserve the right to change this behaviour, or at least make sure that other folks don't have to ponder what the behaviour is. But I'm happy with the change for sure.

nsmith5 reviewed Jan 4, 2022

View reviewed changes

jdolitsky force-pushed the issue-301 branch from ac07095 to d6c41a2 Compare January 5, 2022 16:30

Switch to use fileca in e2e tests

8738de8

Resolves sigstore#301 Signed-off-by: Josh Dolitsky <josh@dolit.ski>

jdolitsky force-pushed the issue-301 branch from d6c41a2 to 8738de8 Compare January 5, 2022 16:37

mattmoor reviewed Jan 5, 2022

View reviewed changes

.github/workflows/verify-k8s.yml Outdated Show resolved Hide resolved

jdolitsky mentioned this pull request Jan 5, 2022

Allow passing configuration via config file #314

Closed

jdolitsky force-pushed the issue-301 branch 4 times, most recently from 676fdb3 to d3c7b0f Compare January 5, 2022 23:30

mattmoor reviewed Jan 5, 2022

View reviewed changes

.github/workflows/verify-k8s.yml Outdated Show resolved Hide resolved

Merge branch 'main' of github.com:sigstore/fulcio into issue-301

ce1b4a6

Signed-off-by: Josh Dolitsky <josh@dolit.ski>

jdolitsky force-pushed the issue-301 branch from d3c7b0f to ce1b4a6 Compare January 6, 2022 15:45

jdolitsky commented Jan 6, 2022

View reviewed changes

mattmoor approved these changes Jan 7, 2022

View reviewed changes

mattmoor merged commit 71f212c into sigstore:main Jan 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to use fileca in e2e tests #309

Switch to use fileca in e2e tests #309

jdolitsky commented Jan 4, 2022

jdolitsky commented Jan 4, 2022

mattmoor commented Jan 4, 2022

nsmith5 commented Jan 4, 2022

nsmith5 Jan 4, 2022

jdolitsky commented Jan 5, 2022

nsmith5 commented Jan 5, 2022

jdolitsky commented Jan 5, 2022

jdolitsky commented Jan 5, 2022

mattmoor Jan 5, 2022

jdolitsky Jan 5, 2022

mattmoor Jan 5, 2022

jdolitsky commented Jan 5, 2022

mattmoor Jan 5, 2022

jdolitsky Jan 6, 2022

jdolitsky Jan 6, 2022

jdolitsky Jan 6, 2022

vaikas Jan 6, 2022

jdolitsky Jan 6, 2022

nsmith5 Jan 6, 2022

nsmith5 Jan 6, 2022

jdolitsky Jan 6, 2022

mattmoor Jan 6, 2022

nsmith5 Jan 7, 2022

nsmith5 Jan 7, 2022

vaikas Jan 7, 2022 •

edited

Loading

Switch to use fileca in e2e tests #309

Switch to use fileca in e2e tests #309

Conversation

jdolitsky commented Jan 4, 2022

jdolitsky commented Jan 4, 2022

mattmoor commented Jan 4, 2022

nsmith5 commented Jan 4, 2022

Choose a reason for hiding this comment

jdolitsky commented Jan 5, 2022

nsmith5 commented Jan 5, 2022

jdolitsky commented Jan 5, 2022

jdolitsky commented Jan 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdolitsky commented Jan 5, 2022

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

vaikas Jan 7, 2022 • edited Loading

Choose a reason for hiding this comment

vaikas Jan 7, 2022 •

edited

Loading