cert-manager / trust-manager

trust-manager is an operator for distributing trust bundles across a Kubernetes cluster.
https://cert-manager.io/docs/projects/trust-manager/
Apache License 2.0
246 stars 66 forks source link

Fix SSA migration field managers #189

Closed erikgb closed 11 months ago

erikgb commented 11 months ago

This PR adapts the SSA migration code to the controller-runtime regression introduced in c/r version 0.15.0 and fixed by https://github.com/kubernetes-sigs/controller-runtime/pull/2435.

/cc @inteon

erikgb commented 11 months ago

/retest

inteon commented 11 months ago

Have you tested running an old trust-manager version to confirm that the field manager is correct? I didn't do this previously but I realised that it is more important than first thought.

erikgb commented 11 months ago

Have you tested running an old trust-manager version to confirm that the field manager is correct? I didn't do this previously but I realised that it is more important than first thought.

Good point. With version 0.5.0 of trust-manager, I get this:

$ k get configmaps -n default example-bundle -o json --show-managed-fields=true | jq '.metadata.managedFields'
[
  {
    "apiVersion": "v1",
    "fieldsType": "FieldsV1",
    "fieldsV1": {
      "f:data": {
        ".": {},
        "f:trust-bundle.pem": {}
      },
      "f:metadata": {
        "f:ownerReferences": {
          ".": {},
          "k:{\"uid\":\"a5814f40-1e51-422c-98b4-890874ddd0b2\"}": {}
        }
      }
    },
    "manager": "trust-manager",
    "operation": "Update",
    "time": "2023-09-28T20:01:21Z"
  }
]

And with version 0.6.0 this:

$ k get configmaps -n default example-bundle -o json --show-managed-fields=true | jq '.metadata.managedFields'
[
  {
    "apiVersion": "v1",
    "fieldsType": "FieldsV1",
    "fieldsV1": {
      "f:data": {
        ".": {},
        "f:trust-bundle.pem": {}
      },
      "f:metadata": {
        "f:ownerReferences": {
          ".": {},
          "k:{\"uid\":\"03db27c7-0cf9-4ae3-a3de-ad95b9eda556\"}": {}
        }
      }
    },
    "manager": "Go-http-client",
    "operation": "Update",
    "time": "2023-09-28T20:08:27Z"
  }
]
erikgb commented 11 months ago

/retest

erikgb commented 11 months ago

/retest

inteon commented 11 months ago

Thanks for finding the reason we were seeing this Go-http-client name in certain older versions & for making the migration path more robust. /approve /lgtm

jetstack-bot commented 11 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inteon

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](https://github.com/cert-manager/trust-manager/blob/main/OWNERS)~~ [inteon] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment