Skip to content

Prepare the next patch release v1.118.0-ske-2#147

Merged
ske-prow[bot] merged 6 commits intoske-v1.118from
patch-release-v1.118
May 14, 2025
Merged

Prepare the next patch release v1.118.0-ske-2#147
ske-prow[bot] merged 6 commits intoske-v1.118from
patch-release-v1.118

Conversation

@timebertt
Copy link
Copy Markdown
Member

How to categorize this PR?

/kind task
/label tide/merge-method-rebase

What this PR does / why we need it:

Apply the latest upstream bug fixes to our fork and revert/replace 4843f1d.

gardener-ci-robot and others added 6 commits May 14, 2025 08:45
backport of gardener#12006
Can be dropped when upgrading to g/g@v1.119

Co-authored-by: Ismail Alidzhikov <9372594+ialidzhikov@users.noreply.github.com>
This reverts commit 4843f1d.

This commit can be dropped along with reverted commit when upgrading to g/g@v1.119.
The workaround has been replaced by gardener#12029 and trusted kubelet
serving certificates on the initial cluster (stackitcloud/ske-base#2891).
…oot-info` ConfigMap (gardener#12049)

backport of gardener#12029
Can be dropped when upgrading to g/g@v1.119

* Revert "Allow the Gardenlet to see the ManagedSeed object of its own cluster (gardener#11965)"

This reverts commit 77cf183 / PR gardener#11965.

The PR gardener#11965 aimed to address a regression in the monitoring stack. However,
the fix was incomplete (1) and incorrect (2). We decided to revert that PR and
follow a simpler approach to achieve the original goal of the PR gardener#9716.

1. Incomplete

   The `ManagedSeed` and `Seed` objects required mutation and reconciliation
   after deploying the fix: PR gardener#11965. Triggering such was not part of the PR.
   Simply deploying the fix is insufficient, and there are no mechanisms that
   regularly trigger the reconciliation of the `ManagedSeed` and `Seed` objects,
   so without a manual mutation and reconciliation (like illustrated below), the
   fix has unfortunately no effect. In some internal landscapes we applied
   these steps manually, but for the general case, we need a better approach.

     for i in $(kubectl get managedseeds --no-headers | awk '{print $1}'); do
       echo $i
       kubectl annotate managedseed $i gardener.cloud/operation=reconcile
     done

     for i in $(kubectl get seeds --no-headers | awk '{print $1}'); do
       echo $i
       kubectl annotate seed $i gardener.cloud/operation=reconcile
     done

2. Incorrect

   The fix introduced a new regression: the `gardenlet` in the managed seed
   started logging errors, indicating that it could not access its own shoot.
   This occurred because the `gardenlet` started to reconcile its own
   `ManagedSeed`, which is conceptually incorrect.

   This happened because with the PR gardener#11773, the network usage of the gardenlet
   was improved. Instead of watching *all* the `ManagedSeed` objects and filter
   out the irrelevant ones in memory in the `ManagedSeed` controller with a
   predicate, a label selector was added to the client on the manager level,
   such that the client is now watching *only* the relevant `ManagedSeed`
   objects and hence causes less network activity and load on the Kubernetes API
   server.

   The regression was that by moving the filtering one layer down, the
   independent monitoring scenario that uses the same client was also affected
   as an undesired and unexpected side effect.

   gardener/observability#51 will help to detect such
   regressions much earlier in the future.

   The monitoring scenario is to GET the `ManagedSeed` object with the same name
   as the seed, to kind of detect that the seed cluster is a Gardener shoot. Due
   to the new label selector filter on the client level, the `ManagedSeed` object
   was no longer visible to the client and seemed to be "not found", which led
   to breaking the monitoring configuration.

   In PR gardener#11965 we added a label to the `ManagedSeed` object such that it could
   pass the label selector filter on the client level and became visible again.
   This fixed the monitoring scenario, but introduced a new regression in the
   `gardenlet`. Namely with PR gardener#11773, the filter on the client level was not
   introduced in addition, but the predicate filter on the controller level was
   removed. So after adding the new label to the `ManagedSeed`, the
   `ManagedSeed` controller started to reconcile the `ManagedSeed` object of the
   `gardenlet` itself, which is conceptually incorrect.

   Having a filter on the client/manager level that is the union of all the
   scenarios of that client, *and* having predicates on the controller level for
   the specific use case would be correct.

Instead of reintroducing the predicate to the `ManagedSeed` controller that was
removed in gardener#11773, we decided to revert PR gardener#11965. A different and even
**simpler** approach will be adopted to achieve the original goal of the PR
kubelet with the Kubernetes Root CA. Instead of checking the `ManagedSeed`
object, we'll check the existence of the `kube-system/shoot-info` configmap.
This is both simpler and correct: for the Kubelet CA decision, it does not
matter how the gardenlet was installed in the seed (via a `ManagedSeed`, via a
helm chart or via any other way). Rather it only matters if the Kubernetes
cluster is a Gardener managed shoot or not.

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com>

* Remove the superfluous label from the managed seeds

PR gardener#11965 attempted to address a regression in the monitoring stack but required
manual mutation and reconciliation of `ManagedSeed` and `Seed` objects after
deployment.

In some Gardener landscapes, `ManagedSeed` objects were manually reconciled to
add the label via the mutating webhook:

    name.seed.gardener.cloud/<seed-name>: "true"

This allowed the gardenlet to access its own `ManagedSeed` object and fixed the
monitoring regression.

However, this PR adopts a different approach, eliminating the need for the
gardenlet to access its own `ManagedSeed`.

This commit removes the no longer necessary label from the `ManagedSeed` objects
that might be present in some installations. The gardener-controller-manager
sends an empty patch during its startup phase to the `ManagedSeed` resource. The
mutating webhook (that was changed in the previous commit) can then delete the
superfluous label.

Without this cleanup, the gardenlet would continue attempting to reconcile its
own `ManagedSeed` object, resulting in error logs, until the `ManagedSeed`
object is mutated for some other reason.

This snippet is added to an existing block of code that also deals with seed
labels and that is scheduled to be deleted after Gardener v1.119 was released.
The scheduled removal after v1.119 is just fine for this scenario as well:

- the regression was introduced in v1.117
- this PR will be regularly released with Gardener v1.119
  - it will be backported to v1.117.x and v1.118.x

So it is fine to remove this code after v1.119.

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com>

* Check if the seed is a shoot via the kube-system/shoot-info ConfigMap

Since PR gardener#11773, the gardenlet can no longer access its own ManagedSeed resource
due to a label selector applied at the manager level.

Instead of reintroducing visibility for the ManagedSeed resource, as attempted
in PR gardener#11965 (and now reverted), we rely on the presence of the
`kube-system/shoot-info` ConfigMap to determine if the seed is a Gardener shoot.

The Kubelet scrape configuration should skip TLS verification if the seed is a
Gardener shoot, as the kubelet's certificate in Gardener shoots is issued by a
separate Kubelet CA.

The original check for the ManagedSeed was not entirely accurate, as it is
irrelevant how the gardenlet is deployed to a Kubernetes cluster. What matters
is whether the cluster is an IaaS Kubernetes cluster (where the kubelet's
certificate is issued by the Kubernetes CA) or a Gardener shoot cluster (where
the kubelet's certificate is issued by a separate Kubelet CA).

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com>

* Rename isManagedSeed to seedIsShoot

The new name aligns better with the related 'seedIsGarden' flag and improves
clarity by explicitly indicating that the seed is a Gardener shoot.

Refer to the previous commit for additional context.

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com>

* Address PR Review: Rename variable

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com>

* Address PR Review: Fix error message

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com

* Address PR Review: Check updated object instead of reading it again

* Address PR Review: Use a simple for loop

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com

* Minor: Remove inline code comment

Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com

* Address PR Review: Remove too simple log messages

Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com

---------

Co-authored-by: Istvan Zoltan Ballok <istvan.zoltan.ballok@sap.com>
Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
Co-authored-by: Victor Herrero Otal <victor.herrero.otal@sap.com>
backport of gardener#12056
Can be dropped when upgrading to g/g@v1.119

Instead of only removing the `kubectl.kubernetes.io/last-applied-configuration`
annotation, we can simply ignore all labels and annotations from the
source object.

Co-authored-by: rfranzke <rafael.franzke@sap.com>
Co-authored-by: Johannes Scheerer <johannes.scheerer@sap.com>
Co-authored-by: Tim Usner <tim.usner@sap.com>
…restricted resources (gardener#12063)

backport of gardener#12051
Can be dropped when upgrading to g/g@v1.119

* Allow gardener-internal to update resources

* Enhance documentation

---------

Co-authored-by: Dimitar Mirchev <dimitar.mirchev@sap.com>
…oadIdentity`s and `Secret`s when used (gardener#12075)

backport of gardener#12061
Can be dropped when upgrading to g/g@v1.119

* Implement dummy `Secret`/`WorkloadIdentity` validator for provider-local

for local testing

Co-Authored-By: Tim Usner <tim.usner@sap.com>

* New webhook for syncing provider secrets

For `WorkoadIdentity`s, there is already
https://github.com/gardener/gardener/blob/b9622262d10177c185e26b4a6551414cc236ac51/plugin/pkg/global/extensionlabels/admission.go#L157-L165
which takes care of syncing the provider label based on the type in the
`spec`.

* Dry-run provider secret creation to ensure it is sane

This way, we ensure that all `*Binding` resources can be considered
"safe to be used" UNDER THE CONDITION that the provider type of the
`*Binding` matches with the provider type of the target resource (e.g.,
a `Shoot` with provider type `foo` must ensure that it only references
a `*Binding` with provider type `foo`).

This is already ensured by https://github.com/gardener/gardener/blob/2a9c566f89a6a16e654193945250f60755c560b9/plugin/pkg/shoot/validator/admission.go#L1073-L1091.
As of today, the `Shoot` API is the only one using `*Binding` resources.

Note that we don't need to do this dry-run-create for
`WorkloadIdentity`s because we already ensure that the provider types of
it and the `CredentialsBinding` match (see previous commit). This way,
we ensure that a `WorkloadIdentity` must have already successfully
passed the provider extension check when referenced in
a `CredentialsBinding` with the same type.

Co-Authored-By: Tim Usner <tim.usner@sap.com>

* New admission plugin preventing undesired finalizer removal of `*Binding`s

Co-Authored-By: Rafael Franzke <rafael.franzke@sap.com>

* Ensure provider types match

* Support `generateName` field

When `CredentialsBinding`s or `SecretBinding`s are created with `generateName`, the `metadata.name` field is not populated, causing the admission plugin to miss validating that the providers between the binding and the shoot match.

In addition, the `ResourceReferenceManager` needs to be a validating admission plugin in order to see the changes applied by `PrepareForCreate`.

* Handle unset `provider` field in `SecretBinding`s

* Address PR review feedback

---------

Co-authored-by: rfranzke <rafael.franzke@sap.com>
Co-authored-by: Tim Usner <tim.usner@sap.com>
@ske-prow ske-prow Bot added kind/task General task tide/merge-method-rebase Denotes a PR that should be rebased by tide when it merges. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 14, 2025
@timebertt
Copy link
Copy Markdown
Member Author

/cc @stackitcloud/ske-gardener

@ske-prow ske-prow Bot requested a review from a team May 14, 2025 07:26
Copy link
Copy Markdown

@maboehm maboehm left a comment

Choose a reason for hiding this comment

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

/hold

@ske-prow ske-prow Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 14, 2025
@ske-prow ske-prow Bot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2025
@ske-prow
Copy link
Copy Markdown

ske-prow Bot commented May 14, 2025

LGTM label has been added.

DetailsGit tree hash: 3cf27fc6ca36c83da4600c426a98f25d963ec411

@ske-prow
Copy link
Copy Markdown

ske-prow Bot commented May 14, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maboehm

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@ske-prow ske-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 14, 2025
@timebertt
Copy link
Copy Markdown
Member Author

/hold cancel

@ske-prow ske-prow Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 14, 2025
@ske-prow ske-prow Bot merged commit 57ffc60 into ske-v1.118 May 14, 2025
4 checks passed
@timebertt timebertt deleted the patch-release-v1.118 branch May 14, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/task General task lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tide/merge-method-rebase Denotes a PR that should be rebased by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants