[GEP-26] Workload Identity support #855

dimityrmirchev · 2024-10-01T09:09:08Z

How to categorize this PR?

/area security ipcei
/kind enhancement
/platform gcp
/label ipcei/workload-identity

What this PR does / why we need it:
This PR adds support for workload identity. A few of the main points of this PR:

A mutator to configure the cloudprovider secret.
A mutator to inject the needed configuration in the Terraformer pod.
MCM container and pod are enhanced with needed configs in order to support this scenario.

Which issue(s) this PR fixes:
Part of gardener/gardener#9586

Special notes for your reviewer:
/cc @vpnachev

In order to develop/review this PR one would need a running Gardener Discovery Server + established trust from GCP account to Gardener's workload identity. This can be achieved using the proposed changes in the following PR: gardener/gardener#10520

The PR also depends on gardener/machine-controller-manager-provider-gcp#130 being reviewed and released.

I will keep this as draft until MCM PR is merged and documentation is added to the PR, but feel free to review/test.

Release note:

The extension now supports `Shoot`s using `WorkloadIdentity`s instead of cloud provider credentials.

gardener-robot · 2024-10-01T09:09:22Z

@dimityrmirchev Label ipcei/workload-identity does not exist.

gardener-robot · 2024-10-01T09:12:11Z

@dimityrmirchev Label ipcei/workload-identity does not exist.

gardener-robot · 2024-10-01T09:37:24Z

@dimityrmirchev Label ipcei/workload-identity does not exist.

gardener-robot · 2024-10-04T15:03:55Z

@dimityrmirchev You need rebase this pull request with latest master branch. Please check.

vpnachev

Some preliminary findings, I will need to take a second look (do not know if the change can be split into several PRs to ease the review).

vpnachev · 2024-10-10T14:20:05Z

pkg/admission/validator/webhook.go

+		},
+		Target: extensionswebhook.TargetSeed,
+		ObjectSelector: &metav1.LabelSelector{
+			MatchLabels: map[string]string{"provider.shoot.gardener.cloud/gcp": "true"},


I do not think the label is set on Workloadidentities, is it?

It is https://github.com/gardener/gardener/blob/00475f59e65c994f01a452aedc472fe0c3887ef5/pkg/controllermanager/controller/credentialsbinding/reconciler.go#L101C2-L112

Ok, but that is valid only for workload identities referenced by credentials binding, otherwise it is not labeled, right?

We discussed with @vpnachev and will replace the LabelSelector. Please see #855 (comment)

vpnachev · 2024-10-10T14:21:04Z

pkg/admission/validator/workloadidentity.go

+	}
+}
+
+// Validate checks whether the given new secret contains a valid GCP service account.


Suggested change

// Validate checks whether the given new secret contains a valid GCP service account.

// Validate checks whether the given new workloadidentity contains a valid GCP configuration.

vpnachev · 2024-10-10T14:24:36Z

pkg/admission/validator/workloadidentity.go

+			return fmt.Errorf("wrong object type %T for old object", oldObj)
+		}
+
+		if workloadIdentity.Spec.TargetSystem.ProviderConfig == nil {


Suggested change

if workloadIdentity.Spec.TargetSystem.ProviderConfig == nil {

if oldWorkloadIdentity.Spec.TargetSystem.ProviderConfig == nil {

But is this check necessary? If it was allowed providerConfig=nil, then there must be a migration path which seems to be forbidden here.

You are right. We can skip the check

vpnachev · 2024-10-10T14:25:58Z

pkg/apis/gcp/types_workloadidentity.go

+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// +genclient


Suggested change

// +genclient

This is not a real API resource, hence a client is not needed.

vpnachev · 2024-10-11T14:26:31Z

On a second look, apart from the comments above, looks good to me.

dimityrmirchev · 2024-10-18T09:56:39Z

Once #866 is merged this PR can be moved to "Ready" state.

dimityrmirchev · 2024-10-21T08:44:38Z

This PR will also need #869 merged so we can use the GardenSecurityProviderType function introduced in g/g v1.104.0.

dimityrmirchev · 2025-01-15T13:09:36Z

I tested this PR after rebasing over the latest changes. The PR is ready for final review.

ping @kon-angelo

dimityrmirchev · 2025-01-15T14:59:56Z

After an offline talk with @vpnachev we decided to explore the possibility of using SubjectTokenSupplier which might replace the internal http server.

dimityrmirchev · 2025-01-17T09:27:29Z

After an offline talk with @vpnachev we decided to explore the possibility of using SubjectTokenSupplier which might replace the internal http server.

I updated the implementation to not rely on a local http server to retrieve the credentials.

gardener-robot added needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) labels Oct 1, 2024

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 1, 2024

gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 1, 2024

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 1, 2024

vpnachev mentioned this pull request Oct 2, 2024

☂️ [GEP-26] Workload Identity - Trust Based Authentication gardener/gardener#9586

Open

53 tasks

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 3, 2024

gardener-robot added the needs/rebase Needs git rebase label Oct 4, 2024

dimityrmirchev force-pushed the workload-id-support branch from b58c085 to f9ae6d3 Compare October 10, 2024 06:38

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 10, 2024

vpnachev reviewed Oct 10, 2024

View reviewed changes

dimityrmirchev force-pushed the workload-id-support branch from f9ae6d3 to efc874d Compare October 22, 2024 09:44

dimityrmirchev marked this pull request as ready for review October 22, 2024 09:44

dimityrmirchev added 11 commits January 15, 2025 14:49

Use Security predicate

40d6a99

Rename useWorkloadIdentityToken to useWorkloadIdentity

ec78643

Properly select workload identities for validation

f7ba6a0

Add test for valid workload identity

f48a1d7

Select all workload identities for validation

14150cf

Typo

778fdf6

Use pointer

2e71953

Configure the backupbucket controller

c70d894

Fix linter findings

b616ceb

Use label selector for workload identities validation

2bb34af

Remove unused const

fff5380

dimityrmirchev force-pushed the workload-id-support branch from 18701fa to fff5380 Compare January 15, 2025 12:49

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 15, 2025

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 15, 2025

Explicitly set min tls version for client and server

0d700b4

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 15, 2025

Rework the way token is retrieved

3ad116c

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 17, 2025

dimityrmirchev added 2 commits January 17, 2025 10:45

Improve config validation

5ac4eab

Fix linter check

7c2eb33

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 17, 2025

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 17, 2025

Typo

11949dc

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 17, 2025

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GEP-26] Workload Identity support #855

[GEP-26] Workload Identity support #855

dimityrmirchev commented Oct 1, 2024 •

edited

Loading

gardener-robot commented Oct 1, 2024

gardener-robot commented Oct 1, 2024

gardener-robot commented Oct 1, 2024

gardener-robot commented Oct 4, 2024

vpnachev left a comment

vpnachev Oct 10, 2024

dimityrmirchev Oct 11, 2024

vpnachev Oct 11, 2024

dimityrmirchev Oct 21, 2024

vpnachev Oct 10, 2024

vpnachev Oct 10, 2024

dimityrmirchev Oct 11, 2024

vpnachev Oct 10, 2024

vpnachev commented Oct 11, 2024

dimityrmirchev commented Oct 18, 2024

dimityrmirchev commented Oct 21, 2024

dimityrmirchev commented Jan 15, 2025

dimityrmirchev commented Jan 15, 2025

dimityrmirchev commented Jan 17, 2025

	// Validate checks whether the given new secret contains a valid GCP service account.
	// Validate checks whether the given new workloadidentity contains a valid GCP configuration.

	if workloadIdentity.Spec.TargetSystem.ProviderConfig == nil {
	if oldWorkloadIdentity.Spec.TargetSystem.ProviderConfig == nil {

[GEP-26] Workload Identity support #855

Are you sure you want to change the base?

[GEP-26] Workload Identity support #855

Conversation

dimityrmirchev commented Oct 1, 2024 • edited Loading

gardener-robot commented Oct 1, 2024

gardener-robot commented Oct 1, 2024

gardener-robot commented Oct 1, 2024

gardener-robot commented Oct 4, 2024

vpnachev 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

vpnachev commented Oct 11, 2024

dimityrmirchev commented Oct 18, 2024

dimityrmirchev commented Oct 21, 2024

dimityrmirchev commented Jan 15, 2025

dimityrmirchev commented Jan 15, 2025

dimityrmirchev commented Jan 17, 2025

dimityrmirchev commented Oct 1, 2024 •

edited

Loading