Kustomise build order gives higher priority to ASO ServiceBus Namespace kind above Kubernetes core API Namespace kind

danielwilsonkainos commented 1 year ago

What happened?

Azure Service Operator (ASO) allows deployment and maintenance of a wide variety of Azure Resources using CRDs in a K8s native way.

One of the resources supported by ASO is ServiceBus. ServiceBus has the conecept of namespacing for queues and topics and as such one of the sub-resources for setting up a ServiceBus is its Namespace.

The CRD for ServiceBus Namespace is as below:

apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: aso-namespace
  namespace: default
spec:
  location: westcentralus
  owner:
    name: aso-sample-rg
  sku:
    name: Standard
  zoneRedundant: false

ServiceBus has its own GroupValue apiVersion however its Kind is Namespace which is the same Kind as K8s core Namespace resource.

In the codebase there is the plugin SortOrderTransformer which has a comparative sort which finalizes ordering for Kustomization.yaml generated using kustomise build.

This transformer plugin contains logic to order by Kind and falls back to a legacy string concatenation of group_value_kind for comparison when resources Kind are the same.

In the scenario when there is a core Kubernetes Namespace resource & a ServiceBus Namespace resource the legacy string concatenation is compared lexicographically. The concatenated strings for each are:

Kuberentes Namespace: ~G_v1_namespace ServiceBus Namespace: servicebus.azure.com_v1beta20210101preview_namespace

When SortOrder for Kustomization is left as default (legacy), the lexiographical sort is causing the ServiceBus to placed at the top of the output Kustomization manifest and the core Namespace below (as ~ has higher unicode value than s). This causes failure as there is no Kubernetes namespace to deploy the ServiceBus Namespace into.

What did you expect to happen?

Core API Namespace should be created first and be placed highest in the Kustomization.yaml output, above ServiceBus

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

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

# core_namespace.yaml
apiVersion: v1
kind: Namespace
metadata:
  name: test-namespace

# servicebus_namespace.yaml
apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: aso-namespace
  namespace: test-namespace
spec:
  location: westcentralus
  owner:
    name: aso-sample-rg
  sku:
    name: Standard
  zoneRedundant: false

Expected output

apiVersion: v1
kind: Namespace
metadata:
  name: test-namespace
---
apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: aso-namespace
  namespace: test-namespace
spec:
  location: westcentralus
  owner:
    name: aso-sample-rg
  sku:
    name: Standard
  zoneRedundant: false

Actual output

apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: aso-namespace
  namespace: test-namespace
spec:
  location: westcentralus
  owner:
    name: aso-sample-rg
  sku:
    name: Standard
  zoneRedundant: false
---
apiVersion: v1
kind: Namespace
metadata:
  name: test-namespace

Kustomize version

5.0.3

Operating system

Other

danielwilsonkainos commented 1 year ago

I have opened a PR with one solution to fix this issue - https://github.com/kubernetes-sigs/kustomize/pull/5458

natasha41575 commented 1 year ago

/assign @ncapps for reproduction and PR review

Thank you for the detailed bug report!

ncapps commented 1 year ago

I was able to reproduce this issue.

/triage accepted

buzzaII commented 1 year ago

Had the simlar issues with Namespace 'kinds':

https://github.com/kubernetes-sigs/kustomize/issues/5192#issue-1745780171

I worked around this by the following (take note of the Kind):

apiVersion: eventhub.azure.com/v1api20211101
kind: NamespaceEH
metadata:
  name: core-eventhub
spec:
  azureName: eventhub
  location: westus
  owner:
    name: core-resourcegroup
  sku:
    name: Standard
    tier: Standard

apiVersion: servicebus.azure.com/v1api20210101preview
kind: NamespaceSB
metadata:
  name: core-servicebus
spec:
  azureName: servicebus
  location: westus
  owner:
    name: core-resourcegroup
  sku:
    name: Premium
  zoneRedundant: false
  operatorSpec:
    secrets:
      primaryConnectionString:
        name: servicebus-secret
        key: serviceBusConnectionString

I then created a Component that fixed the 'kinds':

resources:
  - azure-servicebus.yaml
  - azure-eventhub.yaml

components:
  - ../../../../_components/azure

kind: Component

patches:
  #HACK to fix service bus and eventhub namespace issues
  - patch: |-
      - op: replace
        path: /kind
        value: Namespace
    target:
      group: eventhub.azure.com
      kind: NamespaceEH

  - patch: |-
      - op: replace
        path: /kind
        value: Namespace
    target:
      group: servicebus.azure.com
      kind: NamespaceSB

danielwilsonkainos commented 1 year ago

@buzzaII thank you very much for contributing your work around, we may look to implement similar for the time being

ncapps commented 1 year ago

Hi @danielwilsonkainos,

Have you considered using the sortOptions field?

I found that configuringorder: fifo with the sortOptions field produces an output where the Namespace resource is above the CRD. Please let me know if this configuration works for your use case.

Below is a Kustomization file that uses the sortOptions field.

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- core_namespace.yaml
- servicebus_namespace.yaml

sortOptions:
  order: fifo

Output:

apiVersion: v1
kind: Namespace
metadata:
  name: test-namespace
---
apiVersion: servicebus.azure.com/v1api20210101preview
kind: Namespace
metadata:
  name: aso-namespace
  namespace: test-namespace
spec:
  location: westcentralus
  owner:
    name: aso-sample-rg
  sku:
    name: Standard
  zoneRedundant: false

danielwilsonkainos commented 1 year ago

HI there - yes we are using sortOptions witth FIFO which does work around this issue, unfortunately for us we are using the Flux GOTK components where this issue is also apparent in the Kustomize-Controller. There is a pending merge for Fluxcd release v2.2.0. I thought it best to raise issues in both places.

natasha41575 commented 11 months ago

Thanks all for the discussion!

I agree that the official recommended workaround would be to use sortOptions with FIFO. I do also agree that long-term, we should address the issue by making sure that we order builtins before CRDs.

Thank you for submitting a PR to address this for namespace. Our only concern with that PR is that it only addresses the issue specifically for namespace. If we can address this by putting all builtin types before all CRDs, I think that will be ideal. Here is my suggestion:

We review and accept https://github.com/kubernetes-sigs/kustomize/pull/5458 for fixing this issue specifically for namespace. This is because I think namespace will be one of the more common types where this issue will occur, because a namespace has to be created before any resources in it can be created. It will be good to at least fix the namespace in the short term while we wait for someone to address the more general issue.
We file a follow-up issue (and link it in a TODO in the above PR) reminding us that we need to address this issue more generally by ordering all builtin types before all CRDs by default.

@ncapps WDYT? If you agree, do you mind filing the followup issue? We can also link https://github.com/kubernetes-sigs/kustomize/blob/a0a9bdfe057506233adc6af499ab716d43036263/kyaml/openapi/openapi.go#L94 in that issue; this is a hardcoded list of all builtin types that gets I think gets autogenerated from the openapi data. It is used today to check for namespacability of builtin resources, but we can probably leverage it to check whether a resource is builtin for ordering the output.

ncapps commented 11 months ago

Thank you @natasha41575. I created follow-up issue #5490 and noted this in #5458.

kubernetes-sigs / kustomize