Skip to content

Conversation

@mdbooth
Copy link
Contributor

@mdbooth mdbooth commented Nov 21, 2025

What this PR does / why we need it:

This change has no effect on the output of this kustomization because
the removed configuration was redundant. However, it fixes a bug which
can be triggered when using this kustomization as a base for another
kustomization.

We remove 2 types of configuration. The first is redundant, the second
is incorrect and triggers the bug.

Firstly we remove nameReference configurations which specify references
which are already part of kustomize's defaults. These were found in:

  • config/govmomi/webhook/kustomizeconfig.yaml
  • config/supervisor/webhook/kustomizeconfig.yaml

Secondly we remove all namespace configurations as these were all
incorrect. These directed the namespace transformer to update
webhooks/clientConfig/service/namespace. However, this is not the
intended function of the namespace transformer: it should only set the
namespace directly on objects and allow references to be updated
automatically by nameReference. Configuring it to update a reference
directly leaves kustomize with inconsistent internal state. Depending on
execution order this can cause a subsequent transformation to fail to
update the reference when it makes further changes to the Service
object.

This issue is common amongst multiple providers, likely because it was present in an early version of the kubebuilder templates.

This PR is based on kubernetes-sigs/cluster-api-provider-azure#5982

To confirm that the removed configuration was redundant:

  • Execute make release-manifests-all
  • Copy ./out to /tmp
  • Apply/revert this change
  • Execute make release-manifests-all again
  • Recursively diff the outputs and confirm that they are identical

This change has no effect on the output of this kustomization because
the removed configuration was redundant. However, it fixes a bug which
can be triggered when using this kustomization as a base for another
kustomization.

We remove 2 types of configuration. The first is redundant, the second
is incorrect and triggers the bug.

Firstly we remove nameReference configurations which specify references
which are already part of kustomize's defaults. These were found in:
* config/govmomi/webhook/kustomizeconfig.yaml
* config/supervisor/webhook/kustomizeconfig.yaml

Secondly we remove all namespace configurations as these were all
incorrect. These directed the namespace transformer to update
webhooks/clientConfig/service/namespace. However, this is not the
intended function of the namespace transformer: it should only set the
namespace directly on objects and allow references to be updated
automatically by nameReference. Configuring it to update a reference
directly leaves kustomize with inconsistent internal state. Depending on
execution order this can cause a subsequent transformation to fail to
update the reference when it makes further changes to the Service
object.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 21, 2025
@mdbooth mdbooth changed the title Remove invalid kustomizeconfig 🐛 Remove invalid kustomizeconfig Nov 21, 2025
@chrischdi
Copy link
Member

/test help

@k8s-ci-robot
Copy link
Contributor

@chrischdi: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-vsphere-e2e-govmomi-blocking-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-ci-latest-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-conformance-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-govmomi-upgrade-1-34-1-35-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-blocking-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-conformance-ci-latest-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-conformance-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-upgrade-1-34-1-35-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main
/test pull-cluster-api-provider-vsphere-test-main
/test pull-cluster-api-provider-vsphere-verify-main

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-vsphere-apidiff-main
/test pull-cluster-api-provider-vsphere-janitor-main

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-vsphere-apidiff-main
pull-cluster-api-provider-vsphere-e2e-govmomi-blocking-main
pull-cluster-api-provider-vsphere-e2e-supervisor-blocking-main
pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main
pull-cluster-api-provider-vsphere-test-main
pull-cluster-api-provider-vsphere-verify-main

In response to this:

/test help

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-sigs/prow repository.

@chrischdi
Copy link
Member

Tested via:

kustomize build --load-restrictor LoadRestrictionsNone config/default > $OUTDIR/default.yaml
kustomize build --load-restrictor LoadRestrictionsNone config/supervisor > $OUTDIR/supervisor.yaml

And comparing outputs, there was no diff 👍

/test pull-cluster-api-provider-vsphere-e2e-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-supervisor-main

@chrischdi
Copy link
Member

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 21, 2025
@chrischdi
Copy link
Member

/hold

Pending CI

@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 4f6e44836acdd87448e00682425b21ed74b2fbf8

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi

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:

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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Nov 21, 2025
@chrischdi
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 21, 2025
@k8s-ci-robot k8s-ci-robot merged commit 6d9ba07 into kubernetes-sigs:main Nov 21, 2025
24 of 25 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.15 milestone Nov 21, 2025
@sbueringer
Copy link
Member

Thx!

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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants