Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a workload status command #22

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

meyskens
Copy link
Contributor

@meyskens meyskens commented Nov 15, 2024

cofidectl workload status foo --pod-name foo-pod --namespace bar --trust-zone foo

Initially, this command will take a workload name, with pod name, namespace and trust-zone as flags, and use Maartje's "attest-me" implementation via a debug container to return human-readable cert information with the CLI.

As a follow-up, I'd like to make it simpler to reference the workload (ideally a single argument) and for pod and cluster info be inferred, but that'll depend on how we handle 'workloads' and will need some additional thought.

@meyskens meyskens changed the title [WIP adds workload status command [WIP] adds workload status command Nov 15, 2024
@markgoddard markgoddard marked this pull request as draft November 15, 2024 09:02
@meyskens meyskens force-pushed the workload-info-via-debug-container branch 2 times, most recently from 8059397 to a3612a6 Compare November 16, 2024 13:42
@meyskens meyskens self-assigned this Nov 16, 2024
@meyskens meyskens force-pushed the workload-info-via-debug-container branch 3 times, most recently from d70a56e to ae18958 Compare November 16, 2024 13:55
@mattbates mattbates self-assigned this Nov 21, 2024
@mattbates mattbates force-pushed the workload-info-via-debug-container branch from ae18958 to e27f3f8 Compare November 21, 2024 10:35
@mattbates mattbates added this to the release-0.6.0 milestone Nov 21, 2024
@mattbates mattbates changed the title [WIP] adds workload status command Adds a workload status command Nov 21, 2024
@mattbates mattbates linked an issue Nov 21, 2024 that may be closed by this pull request
@mattbates mattbates marked this pull request as ready for review November 21, 2024 10:37
@mattbates
Copy link
Contributor

mattbates commented Nov 21, 2024

I've merged in main and also (force) pushed some updates.

FYI this PR depends on cofide/cofidectl-debug-container also being opened up. There are small improvements to be made to it (eg some simple docs and a build script), but in the meantime "Cofiders" can build it directly for testing.

Copy link
Contributor

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good

cmd/cofidectl/cmd/workload/workload.go Show resolved Hide resolved
cmd/cofidectl/cmd/workload/workload.go Show resolved Hide resolved
cmd/cofidectl/cmd/workload/workload.go Show resolved Hide resolved
cmd/cofidectl/cmd/workload/workload.go Outdated Show resolved Hide resolved
internal/pkg/workload/workload.go Outdated Show resolved Hide resolved
@mattbates
Copy link
Contributor

@nialdaly thanks for the approval, you've verified everything works for you?

@nialdaly
Copy link
Contributor

It worked well for me:

./cofidectl workload status --namespace test --pod-name client-6c7f5bf7dc-mzkh4 --trust-zone test
✅ Complete: Successfully executed ephemeral debug container in client-6c7f5bf7dc-mzkh4

Trust bundles received
* spiffe://cofide-a.test
    Certificate "CD:11:C2:66:50:4C:A9:D6:9E:99:9B:DA:CB:CE:6A:0F:C1:44:54:53:D4:D8:70:A4:F0:37:D4:FB:40:24:7E:92"
    is a CA certificate
    valid from "2024-11-25T09:49:43Z" to "2024-11-25T21:49:53Z"
    Subject: SERIALNUMBER=326417798281054835965078118436728917784,CN=example.org,O=Example,C=ARPA
    DNS names: spiffe://cofide-a.test
    Signature algorithm: SHA256-RSA
    Issuer: SERIALNUMBER=326417798281054835965078118436728917784,CN=example.org,O=Example,C=ARPA

SVIDs received
* spiffe://cofide-a.test/ns/test/sa/default
    Certificate "80:1F:DA:62:A7:A7:A1:53:A5:D4:DC:08:36:AB:FF:DF:63:55:25:2F:11:4D:EA:99:84:FE:FF:9B:49:C2:6B:43"
    valid from "2024-11-25T10:26:33Z" to "2024-11-25T14:26:43Z"
    Subject: O=SPIRE,C=US
    DNS names: spiffe://cofide-a.test/ns/test/sa/default
    Signature algorithm: SHA256-RSA
    Issuer: SERIALNUMBER=326417798281054835965078118436728917784,CN=example.org,O=Example,C=ARPA
    SVID verified against trust bundle

@jsnctl
Copy link
Contributor

jsnctl commented Nov 28, 2024

Hold off until debug container image hosting story is finalised

@meyskens meyskens force-pushed the workload-info-via-debug-container branch 2 times, most recently from e8d1c72 to 991f0f5 Compare December 19, 2024 16:05
@meyskens meyskens force-pushed the workload-info-via-debug-container branch 2 times, most recently from 8f5eccb to 185fb4d Compare December 19, 2024 16:14
@meyskens
Copy link
Contributor Author

Now using the GHCR image. CI is passing again ready for a last re-read before merging

Copy link
Contributor

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change has gone back to an earlier version prior to addressing review feedback. The commit that was approved was e27f3f8.

cmd/cofidectl/cmd/workload/workload.go Outdated Show resolved Hide resolved
@markgoddard
Copy link
Contributor

I think a bit of git magic will help here:

git checkout e27f3f8
git merge-base origin/main HEAD
git reset --soft $(git merge-base origin/main HEAD)
git status
git diff --cached 
git commit -m "New commit message here"
git rebase origin/main

@meyskens meyskens force-pushed the workload-info-via-debug-container branch 2 times, most recently from f911412 to e8c4a7a Compare December 20, 2024 13:12
@meyskens
Copy link
Contributor Author

@markgoddard git surgery should be done! hope it worked out well...

@markgoddard markgoddard force-pushed the workload-info-via-debug-container branch from e8c4a7a to 332d67c Compare December 23, 2024 17:05
cmd/cofidectl/cmd/workload/workload.go Outdated Show resolved Hide resolved
cmd/cofidectl/cmd/workload/workload.go Outdated Show resolved Hide resolved
@markgoddard
Copy link
Contributor

I reworked it to be closer to the last approved patch and to use the statusspinner.

This change adds a 'cofidectl workload status' command. The command
accepts a trust zone, pod name and namespace, and deploys an ephemeral
debug container to the pod. This container emits diagnostic information
about the SVIDs provided to the workload.

This requires us to set disableContainerSelectors=true in the SPIRE Helm
configuration, to allow the debug container to obtain an ID.

Fixes: #14
Co-Authored-By: Matt Bates <[email protected]>
Co-Authored-By: Maartje Eyskens <[email protected]>
@markgoddard markgoddard force-pushed the workload-info-via-debug-container branch from 332d67c to 8287ec5 Compare December 23, 2024 17:16
@markgoddard markgoddard merged commit d327052 into main Dec 23, 2024
6 checks passed
@markgoddard markgoddard deleted the workload-info-via-debug-container branch December 23, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workload status command
5 participants