kustomize
kustomize copied to clipboard
Issue with Deployment resource patching when Rollout resources openapi definition is included
Describe the bug
We have four directories:
deployment-base - A simple Deployment resource
resources:
- deployment.yaml
deployment-patch - Patch for deployment-base's Deployment resource
resources:
- ../deployment_base
patches:
- deploy_patch.yaml
rollout-base - A Rollout resource with its openapi definition
resources:
- rollout.yaml
openapi:
path: rollout_cr_schema.json
rollout-patch - Patch for rollout-base's Rollout resource
resources:
- ../rollout_base
patches:
- rollout_patch.yaml
If I run kustomize build deployment-patch it works as expected and it does patch the Deployment resource
If I run kustomize build rollout-patch it works as expected and it does patch the Rollout resource
If I run kustomize build with the following kustomization.yaml then it works as expected (and both resources are patched properly):
resources:
- deployment_patch
- rollout_patch
But when I change the sequence to the following
resources:
- rollout_patch
- deployment_patch
then Rollout resource is still patched properly but at Deployment resource the container definition does not contain properties that are defined only in base.
I have prepared a repo with the details and examples: https://github.com/gruberrichard/kustomize-bug-with-openapi
Expected output
I would expect that the generated manifests are the same (and patching of Deployment resource does not break) if I change the sequence of resource directories
Kustomize version
{Version:kustomize/v4.4.1 GitCommit:b2d65ddc98e09187a8e38adc27c30bab078c1dbf BuildDate:2021-11-11T23:36:27Z GoOs:darwin GoArch:amd64}
Platform
macOS
I've just ran into this issue with a Deployment's containers object not merging properly with my openapi definition just containing a Knative service schema. Is it not intended behaviour to merge the definitions provided by openapi: with the builtin definitions? Or do I need to commit the whole output of kustomize openapi fetch to my repository and then manually add the CRD?
I'm facing issues when trying to patch different resources at a time: rollout, services, etc.
Once I specified the openapi.path field to the rollout schema the other components (services) are not merging as expected anymore.
Feeling like there is some built-in schema exists if no openapi.path field is provided. But once you provide your custom one then the build-in schema is not respected anymore.
Is there a way to add/append your custom schema to the existing built-in so all the resources will be patched as expected?
I am having the same issue where when openapi path is added, my deployments are missing specified resource limits that are added with a patch.
If I remove the openapi path then my deployment limit is patched properly.
Argo project will have a workaround for this soon with a blog post detailing how we came about it. But you can peak at this as well https://github.com/argoproj/argo-schema-generator
Thanks @zachaller, I used the https://raw.githubusercontent.com/argoproj/argo-schema-generator/main/schema/argo_all_k8s_kustomize_schema.json contents as my openapi.path and that worked well. My resource limits and requests all rendered correctly.
@dan-j, I also verified that your solution worked. I used the output of kustomize openapi fetch as my openapi.path and everything rendered correctly; however, I think that this assumes that argo has been installed in your cluster previously.
I opted for @zachaller since it seemed clean and also resulted in a smaller schema size.
Thank @zachaller
Those who commented that the openapi field requires the full schema, not just additions, are correct. So it isn't a bug, but there are a few related problems:
- The docs for the
openapifield show it being used to add a schema containing a single definition. While the example works (because its resources only have the type in question), it is not representative of a real use case. - The
openapifield needs to be reconciled with thecrdsfield (which is additive) in general: https://github.com/kubernetes-sigs/kustomize/issues/3944 - The desired behaviour in this issue is essentially a feature request that @natasha41575 and I agreed to in general terms in the above issue: there should be a way to merge the schemas provided in
openapiwith the built-ins ( perhaps abehavior: merge|replacefield, like ConfigMapGenerator?).
/triage accepted /kind feature /kind documentation /retitle OpenAPI field should support merging
@KnVerey FYI, I have an umbrella issue with some of the things I want to do with openapi: https://github.com/kubernetes-sigs/kustomize/issues/4569
It includes a note that the supplied openapi should be an addition to what's builtin, rather than replacing it all.
I have a blog post that should be posted this Thursday (8/25/2022) here that will also go into some of these issues as well. But really looking forward to either changes from kustomize or k8s to help with this issue as well.
For those interested https://blog.argoproj.io/argo-crds-and-kustomize-the-problem-of-patching-lists-5cfc43da288c
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale - Mark this issue or PR as rotten with
/lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Reopen this issue with
/reopen - Mark this issue as fresh with
/remove-lifecycle rotten - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close not-planned
@k8s-triage-robot: Closing this issue, marking it as "Not Planned".
In response to this:
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied- After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied- After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closedYou can:
- Reopen this issue with
/reopen- Mark this issue as fresh with
/remove-lifecycle rotten- Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close not-planned
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/test-infra repository.
/lifecycle frozen
The custom openapi example in the documentation suggests strongly that the openapi keyword merges in the custom schema rather than completely replacing the compiled-in schema.
We should update the example to point out that it is replacing the default schema and with that example, kustomize will no longer correctly handle lists in native resources like pods, deployments, etc.
I agree the docs could be more clear. I noticed under openapi there is no list, just path so I figured all CRDs I want to edit need to be in the same file but didn't imagine it was everything.
I'm going to try and implement a relatively not too complex system for this using kustomize and some text parsing.
If you download all the schemas and store them in their own kustomize directories, attach a random GVK, patch the schemas you want to patch, run a kustomize that calls all the directories, chop off the random GVK and add in definitions at the top you got yourself a patching system for schemas:
$ head rt.openapi.yaml
apiVersion: v12
kind: OpenApi
metadata:
name: io.solo.gateway.v1.RouteTable
def:
io.solo.gateway.v1.RouteTable:
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
$ cat rt.openapi.patch.yaml
apiVersion: v12
kind: OpenApi
metadata:
name: io.solo.gateway.v1.RouteTable
def:
io.solo.gateway.v1.RouteTable:
properties:
spec:
properties:
routes:
x-kubernetes-patch-merge-key: name
x-kubernetes-patch-strategy: merge
$ kust build | yq '.def."io.solo.gateway.v1.RouteTable".properties.spec.properties.routes | keys'
- items
- type
- x-kubernetes-patch-merge-key
- x-kubernetes-patch-strategy
@snuggie12 I've implemented something like that here.