Another potential issue around kind:Namespace

pb-adrianbezzina commented 1 year ago

What happened?

I have the following manifest:

apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: servicebus
spec:
  azureName: servicebus
  location: australiaeast

and with a transformer configured like so:

apiVersion: builtin
kind: PrefixSuffixTransformer
metadata:
  name: AzureNamePostfix
suffix: -devtest
fieldSpecs:
  - path: spec/azureName

The transformation doesn't apply after a kustomize build - and the original manifest is the end result of the build

Seems to be similar to issue #5072 where having a resource of KIND namespace is confusing the kustomize command - if this is found to be a issue - would be wise to check all places there is logic around Kind: Namespace

What did you expect to happen?

Manifest:

apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: servicebus
spec:
  azureName: servicebus
  location: australiaeast

and with a transformer configured like so:

apiVersion: builtin
kind: PrefixSuffixTransformer
metadata:
  name: AzureNamePostfix
suffix: -devtest
fieldSpecs:
  - path: spec/azureName

New Manifest:

apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: servicebus
spec:
  azureName: servicebus-devtest
  location: australiaeast

How can we reproduce it (as minimally and precisely as possible)?

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - resources.yaml
transformers:
  - env.yaml

# resources.yaml
apiVersion: servicebus.azure.com/v1api20210101preview 
kind: Namespace
metadata:
  name: servicebus
spec:
  azureName: servicebus
  location: australiaeast

# env.yaml
apiVersion: builtin
kind: PrefixSuffixTransformer
metadata:
  name: AzureNamePostfix
suffix: -devtest
fieldSpecs:
  - path: spec/azureName

Expected output

apiVersion: servicebus.azure.com/v1api20210101preview 
kind: Namespace
metadata:
  name: servicebus
spec:
  azureName: servicebus-devtest
  location: australiaeast

Actual output

apiVersion: servicebus.azure.com/v1api20210101preview 
kind: Namespace
metadata:
  name: servicebus
spec:
  azureName: servicebus
  location: australiaeast

Kustomize version

5.0.3

Operating system

Linux

natasha41575 commented 1 year ago

/assign @annasong20

annasong20 commented 1 year ago

Hi @pb-adrianbezzina, thank you for the issue report. The problem is that the name suffix transformer is hard-coded to skip all resources with kind: Namespace.

The fix for this issue would be to change the current namespace GVK to reflect only the k8s built-in Namespace object. Like in #5072, this is currently not as simple as specifying the Group and Version fields on the GVK because the built-in Namespace object has an empty group, which the GVK code interprets as any group. If Group is the empty string and Version: v1, the current issue would be fixed, but any custom object with non-empty Group, Version: v1, and kind: Namespace would suffer from the same no-op that this issue describes.

In the short term, we could hard-code this logic into the prefix, suffix transformers, much like in #5133. In the long run, though, I think it may be worth changing isMatchGVK. I agree that kustomize should have built-in knowledge of Namespace, as stated in this https://github.com/kubernetes-sigs/kustomize/issues/5072#issuecomment-1480137735. However, isn't kustomize exercising its knowledge by specifying these GVKs already? It seems more cumbersome hard-coding this logic everywhere we need to use Namespace or similar objects. We should ensure that if we change isMatchGVK, we find an elegant solution that's backward compatible (definitely possible, for example, we could introduce a boolean to dictate if an empty Group is actually empty vs. wildcard).

@natasha41575 would love to get your thoughts on the feasibility of this.

/triage accepted

annasong20 commented 1 year ago

After discussion, we've agreed that we'd consider changes to isMatchGVK IF they aren't breaking.

/unassign

k8s-triage-robot commented 3 months ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

Confirm that this issue is still relevant with /triage accepted (org members only)
Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

k8s-triage-robot commented 6 days ago

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

This bot triages un-triaged issues 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 issue is closed

You can:

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

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

/lifecycle stale

kubernetes-sigs / kustomize