feat: add API types for secrets sync controller #1364

mandreap · 2023-10-29T16:59:54Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Provides the API for the Secret Store Sync Controller.

At the end this feature will:

remove the secret sync controller from the Secret Store CSI driver
create a new secret store sync controller to synchronize secrets as kubernetes secrets
update the RBAC permissions for the Secret Store CSI controllers

Special notes for your reviewer:

This will be moved in a separate project. Added here for review during the next Sig Auth API review meeting.

TODOs:

squashed commits
includes documentation
adds unit tests

k8s-ci-robot · 2023-10-29T17:00:04Z

Hi @mandreap. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

enj

I think the controller's RBAC, admission policy, and CLI config are required to complete the picture on this API.

secretsstoresynccontroller/config/samples/AzureSpcMultipleSecrets.yaml

secretsstoresynccontroller/config/samples/SecretStoreSecret1.yaml

enj · 2023-11-01T19:38:38Z

secretsstoresynccontroller/config/samples/SecretStoreSecret2.yaml

+  secretProviderClassName: spc-multiple-secrets       # [REQUIRED] name of the SecretProviderClass
+  serviceAccountName: <workload-identity-sa-name>     # [REQUIRED] name of the service account


@aramase this does means you could try to use the same SPC across multiple SAs. That sounds like something that is possible today via the pod mount, but does anyone actually do that?

it is used when the same SPC needs to be mounted in 2 different pods that have different SA. I don't think it's done explicitly but because the pods already have a SA configured they would just use that as-is.

secretsstoresynccontroller/api/v1alpha1/secretstoresync_types.go

...tsstoresynccontroller/config/crd/bases/secretsync.secret-sync.x-k8s.io_secretstoresyncs.yaml

aramase · 2023-11-02T16:39:20Z

/retitle feat: add API for secrets sync

secretsstoresynccontroller/api/v1alpha1/secretstoresync_types.go

aramase · 2023-11-02T16:53:49Z

secretsstoresynccontroller/config/samples/SecretStoreSecret2.yaml

+  secretProviderClassName: spc-multiple-secrets       # [REQUIRED] name of the SecretProviderClass
+  serviceAccountName: <workload-identity-sa-name>     # [REQUIRED] name of the service account


it is used when the same SPC needs to be mounted in 2 different pods that have different SA. I don't think it's done explicitly but because the pods already have a SA configured they would just use that as-is.

k8s-ci-robot · 2023-11-02T16:54:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mandreap
Once this PR has been reviewed and has the lgtm label, please assign ritazh for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

…nto mandrea/secretstoresynccontroller

…oller

…nto mandrea/secretstoresynccontroller

…ap/secrets-store-csi-driver into mandrea/secretstoresynccontroller

liggitt

a few other notes at https://docs.google.com/document/d/1k3XxUTEjcQifSXjoy5CcQY9QttdhQ-EG-R3vrKq6SUc/edit?resourcekey=0-5xMYchKvoisABPw9tsR3nA&tab=t.0#heading=h.rjd15w9fa88v

secretsstoresynccontroller/api/v1alpha1/secretstoresync_types.go

...troller/config/admissionpolicies/create_secret_sync_object_exists_for_kubernetes_secret.yaml

secretsstoresynccontroller/config/admissionpolicies/create_update_specific_types.yaml

secretsstoresynccontroller/config/admissionpolicies/deny_token_creation.yaml

...ntroller/config/admissionpolicies/update_secret_sync_object_exists_for_kubernete_secret.yaml

secretsstoresynccontroller/config/controllers/secretstoresync_controller.go

…criptions

secretsstoresynccontroller/config/controllers/secretstoresync_controller.go

secretsstoresynccontroller/api/v1alpha1/secretstoresync_types.go

…nto mandrea/secretstoresynccontroller

liggitt · 2024-02-01T18:32:46Z

secretsstoresynccontroller/api/v1alpha1/secretstoresync_types.go

+	// +kubebuilder:validation:XValidation:message="Labels should have < 63 characters for both keys and values.",rule="(self.all(x, x.size() < 63 && self[x].size() < 63) == true)"
+	// +kubebuilder:validation:XValidation:message="Labels should not contain secrets-store.sync.x-k8s.io. This key is reserved for the controller.",rule="(self.all(x, x.contains('secrets-store.sync.x-k8s.io') == false))"
+	// +optional
+	Labels map[string]string `json:"labels,omitempty"`


@jpbetz @cici37 we ran into a gap here where CEL validation of the key hits budget limits because there's no way to constrain the maxLength of the key in openapi, so the worst-case 1-3MB size is used for the key. This meant a normal regex to check qualified name got rejected as too expensive.

Probably worth opening an issue for kubebuilder to be able to specify constraints on the value (like maxLength). CRDs can express this but it doesn't seem like kubebuilder can.

secretsstoresynccontroller/api/v1alpha1/secretstoresync_types.go

k8s-triage-robot · 2024-05-28T13:07:00Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

mandreap added 2 commits October 29, 2023 16:56

adding secret store sync controller api

6a549a4

timestamp

f02a882

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 29, 2023

k8s-ci-robot requested review from nilekhc and ritazh October 29, 2023 17:00

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 29, 2023

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 29, 2023

mandreap marked this pull request as draft October 29, 2023 17:01

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 29, 2023

mandreap changed the base branch from main to feature/secrets-sync-controller October 30, 2023 12:59

enj reviewed Nov 1, 2023

View reviewed changes

k8s-ci-robot changed the title ~~Mandrea/secretstoresynccontroller~~ feat: add API for secrets sync Nov 2, 2023

aramase requested changes Nov 2, 2023

View reviewed changes

aramase changed the title ~~feat: add API for secrets sync~~ feat: add API types for secrets sync Nov 2, 2023

mandreap added 6 commits November 6, 2023 14:55

update API description and solve comments

d144f2b

Merge branch 'main' of github.com:mandreap/secrets-store-csi-driver i…

e7b88e6

…nto mandrea/secretstoresynccontroller

address pr comments

a2c4b37

complete comment

050bfb5

Merge branch 'kubernetes-sigs:main' into mandrea/secretstoresynccontr…

0c61e9a

…oller

Merge branch 'main' of github.com:mandreap/secrets-store-csi-driver i…

afca7a1

…nto mandrea/secretstoresynccontroller

mandreap changed the title ~~feat: add API types for secrets sync~~ feat: add API types for secrets sync controller Nov 27, 2023

mandreap added 2 commits November 27, 2023 16:22

add policies and update fields

a8dfba1

Merge branch 'mandrea/secretstoresynccontroller' of github.com:mandre…

ef8b0b4

…ap/secrets-store-csi-driver into mandrea/secretstoresynccontroller

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 27, 2023

update comments

715c428

mandreap requested a review from aramase November 29, 2023 10:03

mandreap added 2 commits December 11, 2023 16:08

remove policy

639e5b0

update comment

d992a26

liggitt reviewed Dec 12, 2023

View reviewed changes

mandreap added 3 commits December 20, 2023 11:54

resolving part1 of PR comments about naming conventions and field des…

0142146

…criptions

add validation for strings and maps

99afcb9

update conditions

c009631

enj reviewed Jan 9, 2024

View reviewed changes

mandreap added 5 commits January 9, 2024 20:10

update condition list size

8b11356

update annotation validation

3efa4e0

Merge branch 'main' of github.com:mandreap/secrets-store-csi-driver i…

0499e00

…nto mandrea/secretstoresynccontroller

update policies

24f262f

update policies

b355bd9

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 31, 2024

mandreap added 8 commits January 31, 2024 15:24

update conditions

9ce5e9b

add regex validation

f886fc9

adding helm versions for the policies

4737440

simplify delete

8e2784b

add validation for labels

0de0f28

apply label validation for update

7d48289

add annotation validation

135fcab

update comments

77c27a5

liggitt reviewed Feb 1, 2024

View reviewed changes

enj mentioned this pull request Feb 5, 2024

CEL in Admission(ValidatingAdmissionPolicy): explicitly excluding resources from validatingadmissionpolicy interception kubernetes/kubernetes#122205

Closed

mandreap added 4 commits February 16, 2024 14:53

updating conditions and policies

5d1c9a6

update prefix

2a39a21

update create policy

2a79912

update policies with ownership

86b57cc

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add API types for secrets sync controller #1364

feat: add API types for secrets sync controller #1364

mandreap commented Oct 29, 2023 •

edited

k8s-ci-robot commented Oct 29, 2023

enj left a comment

enj Nov 1, 2023

aramase Nov 2, 2023

aramase commented Nov 2, 2023

aramase Nov 2, 2023

k8s-ci-robot commented Nov 2, 2023

liggitt left a comment

liggitt Feb 1, 2024

liggitt Feb 1, 2024

k8s-triage-robot commented May 28, 2024

		secretProviderClassName: spc-multiple-secrets # [REQUIRED] name of the SecretProviderClass
		serviceAccountName: <workload-identity-sa-name> # [REQUIRED] name of the service account

feat: add API types for secrets sync controller #1364

Are you sure you want to change the base?

feat: add API types for secrets sync controller #1364

Conversation

mandreap commented Oct 29, 2023 • edited

k8s-ci-robot commented Oct 29, 2023

enj left a comment

Choose a reason for hiding this comment

enj Nov 1, 2023

Choose a reason for hiding this comment

aramase Nov 2, 2023

Choose a reason for hiding this comment

aramase commented Nov 2, 2023

aramase Nov 2, 2023

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 2, 2023

liggitt left a comment

Choose a reason for hiding this comment

liggitt Feb 1, 2024

Choose a reason for hiding this comment

liggitt Feb 1, 2024

Choose a reason for hiding this comment

k8s-triage-robot commented May 28, 2024

mandreap commented Oct 29, 2023 •

edited