Workload Identity reconciler integration using SPIFFE #809

PrimalPimmy · 2024-09-17T10:19:54Z

This PR initiates the work to implement workload Identity in the nephio ecosystem. More documentation can be found here:
Design Document: https://docs.google.com/document/d/1k8Hcd7tJKPIsyiYZX6hpRECuJ4IIxVnaESghU5bLNVQ/edit?usp=sharing
User Story: https://docs.google.com/document/d/1nkh7tTItwii1bY877PfzjFCBtmRos4IDh5EOJxWXRdg/edit?usp=sharing

nephio-prow · 2024-09-17T10:20:06Z

Hi @PrimalPimmy. Thanks for your PR.

I'm waiting for a nephio-project member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

liamfallon · 2024-09-17T10:53:50Z

/ok-to-test

liamfallon · 2024-09-20T09:17:33Z

/assign @tliron @efiacor @kispaljr

liamfallon · 2024-09-20T09:18:22Z

@PrimalPimmy I think maybe we should take another look at this in SIG-Auto. Would you be able to schedule it on the agenda for one of the upcoming meetings?

PrimalPimmy · 2024-09-20T19:31:55Z

Sure @liamfallon . Where do I post about this to schedule it?

cc: @nyrahul

liamfallon · 2024-09-23T08:46:47Z

Please enter an item on the agenda for a forthcoming meeting, the agenda document is here:
https://docs.google.com/document/d/1SW4acc0950QdDNEvmeHArfNsgKPeY_EIwHBqGBky2CY/edit#heading=h.k7dq727kte8c

efiacor · 2025-01-22T09:39:15Z

@PrimalPimmy is this PR ready for review?
We will need to add some testing, remove any dead/commented out code and get the presubmit jobs passing.

liamfallon · 2025-01-29T10:56:46Z

/retest

PrimalPimmy · 2025-02-11T15:57:06Z

I'm not sure why the CI is unable to check go.sum for the latest spiffe packages, even though I had updated using go mod tidy in the controllers/pkg directory

efiacor · 2025-02-11T16:13:57Z

I'm not sure why the CI is unable to check go.sum for the latest spiffe packages, even though I had updated using go mod tidy in the controllers/pkg directory

Some of the make targets should be executed from the root of the project.

make tidy
make fmt
etc

Can you run "make test" successfully on local?

PrimalPimmy · 2025-02-11T16:28:47Z

Thanks @efiacor , the tests pass now

liamfallon

/approve

efiacor

We really should have at least some degree of unit testing. At least on code that doesn't require heavy mocking/stubbing.

operators/nephio-controller-manager/main.go

controllers/pkg/resource/workloadapi.go

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go

controllers/pkg/go.mod

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go

liamfallon · 2025-02-18T07:06:49Z

/remove-approve

PrimalPimmy · 2025-02-18T09:46:32Z

A rough flow of how it is working now:

Make sure when a new cluster is formed, make sure this secret nephio/optional/spire-restrictedSA/Secret.yaml (https://github.com/nephio-project/catalog/pull/84/files#diff-884d3b1eb631f5d33f1346355a00a9fc38767dc982ec4d00ba7017d4022bbab6) and all of it's RoleBindings are applied.
Using the above SA token created, the reconciler will check for it, and will generated a Kubeconfig configmap. This kubeconfig configmap will then be used for cluster/node attestation.

controllers/pkg/go.mod

efiacor · 2025-02-18T12:22:25Z

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go

+					return ctrl.Result{RequeueAfter: 30 * time.Second}, errors.Wrap(err, msg)
+				}
+				if !ready {
+					log.Info("cluster not ready")


Log error instead of Info

I was following this:
https://github.com/nephio-project/nephio/blob/main/controllers/pkg/reconcilers/bootstrap-secret/reconciler.go#L129

, and other reconcilers didn't use log.error for cluster readiness either.

Ok, leave it at Info so.

Another quick one, I see that most of the following steps we only log the error but continue on. Should we be exiting if any of the "logged" errors below occur?

Yep, I think we should exit them. Do we exit with return ctrl.Result{}, errors.Wrap(err, msg)?

Not sure tbh. That would be down to the design. Do you want to result.RequeueAfter or let the default backoff handle it. Question for someone with more k8s knowledge.

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go

efiacor · 2025-02-18T12:26:59Z

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go

+	token := string(secret.Data["token"])
+
+	// Retrieve the cluster's CA certificate
+	configMap, err := clientset.CoreV1().ConfigMaps("kube-system").Get(ctx, "kube-root-ca.crt", metav1.GetOptions{})


A lot of the inputs here are hardcoded. This is bad practice.
All the inputs should be configurable via configmap or args.

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go

efiacor · 2025-02-18T12:30:13Z

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go

+	// Get the ConfigMap
+	cm := &v1.ConfigMap{}
+	if err := r.Get(ctx, types.NamespacedName{
+		Namespace: "spire",


Again, hardcoded inputs should be avoided.

efiacor · 2025-02-18T12:31:27Z

controllers/pkg/resource/spiffeutils.go

This can prob go beside the reconciler tbh. No real use putting it in the resources dir.

efiacor · 2025-02-18T12:31:58Z

controllers/pkg/resource/spiffeutils.go

+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func CreateSpireAgentConfigMap(name string, namespace string, cluster string, serverAddress string, serverPort string) (*v1.ConfigMap, error) {


These functions should be relatively easy to unit test.

Signed-off-by: PrimalPimmy <[email protected]>

PrimalPimmy · 2025-02-18T15:42:11Z

/test presubmit-nephio-golangci-lint

liamfallon · 2025-02-18T16:09:59Z

If you add a comment with a reference to issue with the refactoring actions for R5 as a comment here,w e can merge this PR.

PrimalPimmy · 2025-02-18T16:20:51Z

Future work in R5 related to SPIFFE:
#859

These are the immediate things to be handled, might add more in the future.

efiacor · 2025-02-18T16:35:57Z

/approve
/lgtm

nephio-prow · 2025-02-18T16:36:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: efiacor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [efiacor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

nephio-prow bot added the do-not-merge/work-in-progress label Sep 17, 2024

nephio-prow bot requested review from henderiw and liamfallon September 17, 2024 10:19

nephio-prow bot added the needs-ok-to-test label Sep 17, 2024

nephio-prow bot added ok-to-test and removed needs-ok-to-test labels Sep 17, 2024

PrimalPimmy force-pushed the spire-fixes branch from b1b3497 to 60bb4c8 Compare September 20, 2024 02:13

nephio-prow bot assigned efiacor, kispaljr and tliron Sep 20, 2024

PrimalPimmy mentioned this pull request Nov 19, 2024

Add SPIRE/SPIFFE Packages for Workload Identity nephio-project/catalog#84

Merged

PrimalPimmy marked this pull request as ready for review January 21, 2025 14:38

nephio-prow bot removed the do-not-merge/work-in-progress label Jan 21, 2025

nephio-prow bot requested a review from efiacor January 21, 2025 14:38

PrimalPimmy force-pushed the spire-fixes branch from 10a28f1 to 3b9bd0c Compare February 11, 2025 16:31

liamfallon reviewed Feb 11, 2025

View reviewed changes

nephio-prow bot added the approved label Feb 11, 2025

efiacor added enhancement New feature or request sig/automation labels Feb 12, 2025

efiacor added the area/security label Feb 12, 2025

efiacor reviewed Feb 12, 2025

View reviewed changes

PrimalPimmy commented Feb 17, 2025

View reviewed changes

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go Outdated Show resolved Hide resolved

PrimalPimmy commented Feb 17, 2025

View reviewed changes

controllers/pkg/reconcilers/spire-bootstrap/reconciler.go Show resolved Hide resolved

nephio-prow bot removed the approved label Feb 18, 2025

SPIFFE Workload Identity Implementation

2b7ffac

PrimalPimmy force-pushed the spire-fixes branch from 8b1896e to 2b7ffac Compare February 18, 2025 11:46

efiacor reviewed Feb 18, 2025

View reviewed changes

PrimalPimmy added 5 commits February 18, 2025 18:25

Log info improvements

cb88125

Signed-off-by: PrimalPimmy <[email protected]>

Adding tests

1e4dfb5

Signed-off-by: PrimalPimmy <[email protected]>

Exiting after errors

dfeec03

Signed-off-by: PrimalPimmy <[email protected]>

change spiffeutils location

954d386

Signed-off-by: PrimalPimmy <[email protected]>

Add functions to spiffeutils

06a0de1

Signed-off-by: PrimalPimmy <[email protected]>

nephio-prow bot added the lgtm label Feb 18, 2025

nephio-prow bot added the approved label Feb 18, 2025

nephio-prow bot merged commit c034605 into nephio-project:main Feb 18, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workload Identity reconciler integration using SPIFFE #809

Workload Identity reconciler integration using SPIFFE #809

PrimalPimmy commented Sep 17, 2024 •

edited

Loading

nephio-prow bot commented Sep 17, 2024

liamfallon commented Sep 17, 2024

liamfallon commented Sep 20, 2024

liamfallon commented Sep 20, 2024

PrimalPimmy commented Sep 20, 2024

liamfallon commented Sep 23, 2024

efiacor commented Jan 22, 2025 •

edited

Loading

liamfallon commented Jan 29, 2025

PrimalPimmy commented Feb 11, 2025

efiacor commented Feb 11, 2025

PrimalPimmy commented Feb 11, 2025

liamfallon left a comment

efiacor left a comment

liamfallon commented Feb 18, 2025

PrimalPimmy commented Feb 18, 2025 •

edited

Loading

efiacor Feb 18, 2025

PrimalPimmy Feb 18, 2025

efiacor Feb 18, 2025

PrimalPimmy Feb 18, 2025

efiacor Feb 18, 2025

efiacor Feb 18, 2025

efiacor Feb 18, 2025

efiacor Feb 18, 2025

PrimalPimmy Feb 18, 2025

efiacor Feb 18, 2025

PrimalPimmy commented Feb 18, 2025

liamfallon commented Feb 18, 2025

PrimalPimmy commented Feb 18, 2025

efiacor commented Feb 18, 2025

nephio-prow bot commented Feb 18, 2025

Workload Identity reconciler integration using SPIFFE #809

Workload Identity reconciler integration using SPIFFE #809

Conversation

PrimalPimmy commented Sep 17, 2024 • edited Loading

nephio-prow bot commented Sep 17, 2024

liamfallon commented Sep 17, 2024

liamfallon commented Sep 20, 2024

liamfallon commented Sep 20, 2024

PrimalPimmy commented Sep 20, 2024

liamfallon commented Sep 23, 2024

efiacor commented Jan 22, 2025 • edited Loading

liamfallon commented Jan 29, 2025

PrimalPimmy commented Feb 11, 2025

efiacor commented Feb 11, 2025

PrimalPimmy commented Feb 11, 2025

liamfallon left a comment

Choose a reason for hiding this comment

efiacor left a comment

Choose a reason for hiding this comment

liamfallon commented Feb 18, 2025

PrimalPimmy commented Feb 18, 2025 • edited Loading

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

PrimalPimmy commented Feb 18, 2025

liamfallon commented Feb 18, 2025

PrimalPimmy commented Feb 18, 2025

efiacor commented Feb 18, 2025

nephio-prow bot commented Feb 18, 2025

PrimalPimmy commented Sep 17, 2024 •

edited

Loading

efiacor commented Jan 22, 2025 •

edited

Loading

PrimalPimmy commented Feb 18, 2025 •

edited

Loading