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

feat: support immutable secrets (#574) #1395

Merged
merged 1 commit into from
Dec 22, 2023
Merged

Conversation

gavinkflam
Copy link
Contributor

@gavinkflam gavinkflam commented Nov 28, 2023

Description of the change

This adds support to seal and unseal immutable secrets via spec.template.immutable. I have also updated a related integration test and did some additional manual testing.

Additional manual testing details

Test 1: Use kubeseal to seal an immutable secret
Result: produced a SealedSecret with spec.template.immutable: true

$ kubeseal <<EOF > sealed.json
apiVersion: v1
kind: Secret
metadata:
  name: test
immutable: true
data:
  foo: deadbeef
EOF

$ cat sealed.json
{
  "kind": "SealedSecret",
  "apiVersion": "bitnami.com/v1alpha1",
  "metadata": {
    "name": "test",
    "namespace": "default",
    "creationTimestamp": null
  },
  "spec": {
    "template": {
      "metadata": {
        "name": "test",
        "namespace": "default",
        "creationTimestamp": null
      },
      "immutable": true
    },
    "encryptedData": {
      "foo": "AgAUYqFKbJ4sKRxs3+Gq24OxYeHDeo2U37wM7hFFQLSDg1cm2A6JVDREm/WAnLRUCzEpqwdlwVJ6HKpGndeepe597zlw+OT16uL4pzOth0dKce8KpM3M5V0dZYOLZ+Apq8LvVJ4l4GV/yaNlLhUV2msgzBzl+mvCo9ge10KVNN70tr0HkYA7m1L51J7py7GTcccp5ytXvIWI2i2xWZ73NlteMGYDnVYwOC7GGA6YL17LNMNpIZoDlfYqcQl0HgGCgwX2VYTlA+WJ9rftfBsvAcwkA8/SSu69r0cYX2BZcG2EI1RlP2mn86Ki4VuqbYPLV1a4e0WfCSchlmHsVUEL/doC43bV7qPcqA60/+kg9hZHTopNYmr8kpIh4Cto4caPOgbT+85+rSsADJT2D7506jxkU4eEEn3AN64Pi8LjcKU8JCHBXsMNlzQ/fKmS1ciJqFD17eihx2IVRMFgBKTgfZsAcBBF6KnPu+J6WDqBvwzoV68cIx/3DYJZUG/tXnx3RZdhxDoYS0bghyA8w+9+6yYimnxqqaAAVQGf/npGm5+eHb+b/Sk6ZD3mpZ4C+eNRsDRCzVh8rcc1yXByuXMSIyw4+3f7opD+MVf4ab/OyOBtHX4/zPku6wQuwdDJO26skj6NJU0zANz2VUg4+ctIwIoIFLn8F0b1PDsBhX2ntShX42ZLqwP41CMs4o67gdAtD3yCy7RtUgM="
    }
  }
}

Test 2: Create a SealedSecret resource with spec.template.immutable: true
Result: the secret is unsealed correctly

$ kubectl apply -f sealed.json
sealedsecret.bitnami.com/test configured

$ kubectl get secret test -o yaml
apiVersion: v1
data:
  foo: deadbeef
immutable: true
kind: Secret
metadata:
  creationTimestamp: "2023-11-28T03:47:27Z"
  name: test
  namespace: default
  ownerReferences:
  - apiVersion: bitnami.com/v1alpha1
    controller: true
    kind: SealedSecret
    name: test
    uid: 0e09c7ed-5a29-40f9-9ea1-ee8e5236951c
  resourceVersion: "42712"
  uid: 10ef189b-7ca5-4777-906f-fde6842c558b
type: Opaque

Benefits

Users can opt-in to create immutable secrets. The new field is optional and the default behavior is not changed.

The benefits of immutable secrets are discussed in the official Kubernetes documentation. https://kubernetes.io/docs/concepts/configuration/secret/#secret-immutable

Possible drawbacks

None.

Applicable issues

Additional information

Requires a release of both Sealed Secrets and the Helm chart

@gavinkflam
Copy link
Contributor Author

Hi maintainers, the pipeline was passing before rebasing. It is now complaining about the execution graph https://cp.bromelia.vmware.com/v1/execution-graphs/5d5b9520-6d8f-4dc4-bf27-6ad12b101552.
Is this a known issue?

Running execution graph: https://cp.bromelia.vmware.com/v1/execution-graphs/5d5b9520-6d8f-4dc4-bf27-6ad12b101552
  Execution graph in progress, will check in 30s.
  Error: Task helm-lint with ID 2a70f69c-f4de-4171-b4fd-bb4990cc96[24](https://github.com/bitnami-labs/sealed-secrets/actions/runs/7054830757/job/19204311440?pr=1395#step:3:26) has failed. Error: Internal task execution handling error
  Error: Task helm-package with ID 70e6d981-0eba-4980-9381-544792032075 has failed. Error: Internal task execution handling error
Processing resulting execution graph...
  Warning: Error downloading bundle files for execution graph 5d5b9520-6d8f-4dc4-bf[27](https://github.com/bitnami-labs/sealed-secrets/actions/runs/7054830757/job/19204311440?pr=1395#step:3:30)-6ad12b101552, error: TypeError: Converting circular structure to JSON
      --> starting at object with constructor 'TLSSocket'
      |     property 'parser' -> object with constructor 'HTTPParser'
      --- property 'socket' closes the circle
  Error: Execution graph 5d5b9520-6d8f-4dc4-bf27-6ad12b101552 has failed.

@alvneiayu
Copy link
Collaborator

hi @gavinkflam

It was a temporal error in the service that we are using to verify the PR. Now, it is green. No worries at all.

We will come back soon to do the review to your changes and thanks a lot for your contribution.

Thanks

Álvaro

@alvneiayu
Copy link
Collaborator

hi @gavinkflam

First, thanks a lot for your contribution. Really awesome job.

During the review, we have detected something that we have some concern. The scenario that we have concern is when you decide to update your Sealed Secrets without deleting it, just update it. For this scenario, the controller will update the secret and I think that you will see errors in the controller several times for the retries. Could you verify it, please? If yes, we need to handle properly this showing a clear error for the users.

Moreover, it would be awesome to include a test with this scenario.

Thanks a lot

Álvaro

* add spec.template.immutable field to the SealedSecrets CRD
* enable controller to unseal immutable secrets
* enable kubeseal to seal immutable secrets
* add error handling for mutating an immutable secret

Signed-off-by: Gavin Lam <[email protected]>
@gavinkflam
Copy link
Contributor Author

Thanks @alvneiayu for reviewing the changes. I have added the following and some tests.

  1. Do not retry updating immutable fields of an immutable secret
  2. Better error message and event message

Copy link
Collaborator

@alvneiayu alvneiayu left a comment

Choose a reason for hiding this comment

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

LGTM, and thanks a lot for your contribution @gavinkflam

@alvneiayu alvneiayu merged commit f7196bb into bitnami-labs:main Dec 22, 2023
17 checks passed
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.

Cannot create immutable secrets
2 participants