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

Another potential issue around kind:Namespace #5192

Open
pb-adrianbezzina opened this issue Jun 7, 2023 · 3 comments
Open

Another potential issue around kind:Namespace #5192

pb-adrianbezzina opened this issue Jun 7, 2023 · 3 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@pb-adrianbezzina
Copy link

pb-adrianbezzina commented Jun 7, 2023

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

@pb-adrianbezzina pb-adrianbezzina added the kind/bug Categorizes issue or PR as related to a bug. label Jun 7, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 7, 2023
@pb-adrianbezzina pb-adrianbezzina changed the title Another issue around kind:Namespace Another potential issue around kind:Namespace Jun 7, 2023
@natasha41575
Copy link
Contributor

/assign @annasong20

@annasong20
Copy link
Contributor

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 #5072 (comment). 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

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 24, 2023
@annasong20
Copy link
Contributor

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

/unassign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

4 participants