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

Invalid Kustomize generation #3425

Closed
lauchokyip opened this issue May 21, 2023 · 6 comments · Fixed by #3456
Closed

Invalid Kustomize generation #3425

lauchokyip opened this issue May 21, 2023 · 6 comments · Fixed by #3456
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@lauchokyip
Copy link
Contributor

lauchokyip commented May 21, 2023

What broke? What's expected?

This PR (#3374) breaks the kustomization files generated. Had to revert back to patchesStrategicMerge

To reproduce:
1)

mkdir testing
cd testing
kubebuilder init --domain tutorial.kubebuilder.io --repo tutorial.kubebuilder.io/project
kubebuilder create api --group batch --version v1 --kind CronJob
kubebuilder create webhook --group batch --version v1 --kind CronJob  --conversion
make manifests
  1. Go to /config/crd/kustomization.yaml and uncomment
patches:
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
# patches here are for enabling the conversion webhook for each CRD
- patches/webhook_in_cronjobs.yaml
#+kubebuilder:scaffold:crdkustomizewebhookpatch

# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
# patches here are for enabling the CA injection for each CRD
- patches/cainjection_in_cronjobs.yaml
#+kubebuilder:scaffold:crdkustomizecainjectionpatch
  1. Run kustomize against it
kustomize build config/crd
Error: invalid Kustomization: json: cannot unmarshal string into Go struct field Kustomization.patches of type types.Patc

Reproducing this issue

No response

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"v3.10.0-57-g959952de-dirty", KubernetesVendor:"unknown", GitCommit:"959952decffecb9a35bb843e706849586883360c", BuildDate:"2023-05-16T02:27:25Z", GoOs:"linux", GoArch:"amd64"}

PROJECT version

No response

Plugin versions

No response

Other versions

No response

Extra Labels

No response

@lauchokyip lauchokyip added the kind/bug Categorizes issue or PR as related to a bug. label May 21, 2023
@camilamacedo86
Copy link
Member

@Sajiyah-Salat,

We need to check the kustomize documentation and see what is missing to get it fixed.
Otherwise we need to revert prior release.

@camilamacedo86 camilamacedo86 added triage/accepted Indicates an issue or PR is ready to be actively worked on. release-blocker labels May 23, 2023
@erikgb
Copy link
Contributor

erikgb commented Jun 13, 2023

I think we are just missing a key in the patches array elements. It should look like this, I think:

patches:
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
# patches here are for enabling the conversion webhook for each CRD
- path: patches/webhook_in_cronjobs.yaml
#+kubebuilder:scaffold:crdkustomizewebhookpatch

# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
# patches here are for enabling the CA injection for each CRD
- path: patches/cainjection_in_cronjobs.yaml
#+kubebuilder:scaffold:crdkustomizecainjectionpatch

@Sajiyah-Salat
Copy link
Contributor

Sorry @camilamacedo86 I was unaware of the issue and now looks like @mjlshen have already done a great work. Thank you for letting me know. I will be careful from next time 😄 .

@camilamacedo86
Copy link
Member

Hi @lauchokyip and @mjlshen,

I am unable to reproduce this issue with the changes in the master branch.
Also, note that I added tests to check this one and all is passing as well: #3468

By looking at the steps you might did not uncomment all that is required.
See that to enable the Webhooks and CertManager you ONLY uncommented the following:

Go to /config/crd/kustomization.yaml and uncomment
patches:
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
# patches here are for enabling the conversion webhook for each CRD
- patches/webhook_in_cronjobs.yaml
#+kubebuilder:scaffold:crdkustomizewebhookpatch

# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
# patches here are for enabling the CA injection for each CRD
- patches/cainjection_in_cronjobs.yaml
#+kubebuilder:scaffold:crdkustomizecainjectionpatch
```
 
When you either must to uncomment all relative places:

**Example**

```
# Adds namespace to all resources.
namespace: testing-system

# Value of this field is prepended to the
# names of all resources, e.g. a deployment named
# "wordpress" becomes "alices-wordpress".
# Note that it should also match with the prefix (text before '-') of the namespace
# field above.
namePrefix: testing-

# Labels to add to all resources and selectors.
#labels:
#- includeSelectors: true
#  pairs:
#    someName: someValue

resources:
- ../crd
- ../rbac
- ../manager
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
# crd/kustomization.yaml
- ../webhook
# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required.
- ../certmanager
# [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'.
#- ../prometheus

patchesStrategicMerge:
# Protect the /metrics endpoint by putting it behind auth.
# If you want your controller-manager to expose the /metrics
# endpoint w/o any authn/z, please comment the following line.
- manager_auth_proxy_patch.yaml

# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
# crd/kustomization.yaml
- manager_webhook_patch.yaml

# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'.
# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks.
# 'CERTMANAGER' needs to be enabled to use ca injection
- webhookcainjection_patch.yaml

 [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix.
 Uncomment the following replacements to add the cert-manager CA injection annotations
replacements:
  - source: # Add cert-manager annotation to ValidatingWebhookConfiguration, MutatingWebhookConfiguration and CRDs
      kind: Certificate
      group: cert-manager.io
      version: v1
      name: serving-cert # this name should match the one in certificate.yaml
      fieldPath: .metadata.namespace # namespace of the certificate CR
    targets:
      - select:
          kind: ValidatingWebhookConfiguration
        fieldPaths:
          - .metadata.annotations.[cert-manager.io/inject-ca-from]
        options:
          delimiter: '/'
          index: 0
          create: true
      - select:
          kind: MutatingWebhookConfiguration
        fieldPaths:
          - .metadata.annotations.[cert-manager.io/inject-ca-from]
        options:
          delimiter: '/'
          index: 0
          create: true
      - select:
          kind: CustomResourceDefinition
        fieldPaths:
          - .metadata.annotations.[cert-manager.io/inject-ca-from]
        options:
          delimiter: '/'
          index: 0
          create: true
  - source:
      kind: Certificate
      group: cert-manager.io
      version: v1
      name: serving-cert # this name should match the one in certificate.yaml
      fieldPath: .metadata.name
    targets:
      - select:
          kind: ValidatingWebhookConfiguration
        fieldPaths:
          - .metadata.annotations.[cert-manager.io/inject-ca-from]
        options:
          delimiter: '/'
          index: 1
          create: true
      - select:
          kind: MutatingWebhookConfiguration
        fieldPaths:
          - .metadata.annotations.[cert-manager.io/inject-ca-from]
        options:
          delimiter: '/'
          index: 1
          create: true
      - select:
          kind: CustomResourceDefinition
        fieldPaths:
          - .metadata.annotations.[cert-manager.io/inject-ca-from]
        options:
          delimiter: '/'
          index: 1
          create: true
  - source: # Add cert-manager annotation to the webhook Service
      kind: Service
      version: v1
      name: webhook-service
      fieldPath: .metadata.name # namespace of the service
    targets:
      - select:
          kind: Certificate
          group: cert-manager.io
          version: v1
        fieldPaths:
          - .spec.dnsNames.0
          - .spec.dnsNames.1
        options:
          delimiter: '.'
          index: 0
          create: true
  - source:
      kind: Service
      version: v1
      name: webhook-service
      fieldPath: .metadata.namespace # namespace of the service
    targets:
      - select:
          kind: Certificate
          group: cert-manager.io
          version: v1
        fieldPaths:
          - .spec.dnsNames.0
          - .spec.dnsNames.1
        options:
          delimiter: '.'
          index: 1
          create: true

```

See the full steps performed locally:

```sh
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ kubebuilder init 
Writing kustomize manifests for you to edit...
Writing scaffold for you to edit...
Get controller runtime:
$ go get sigs.k8s.io/controller-runtime@v0.15.0
Update dependencies:
$ go mod tidy
Next: define a resource with:
$ kubebuilder create api
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ kubebuilder create api --group batch --version v1 --kind CronJob
Create Resource [y/n]
y
Create Controller [y/n]
y
Writing kustomize manifests for you to edit...
Writing scaffold for you to edit...
api/v1/cronjob_types.go
api/v1/groupversion_info.go
internal/controller/suite_test.go
internal/controller/cronjob_controller.go
Update dependencies:
$ go mod tidy
Running make:
$ make generate
mkdir -p /Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin
test -s /Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen && /Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen --version | grep -q v0.12.0 || \
	GOBIN=/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.12.0
/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
Next: implement your new API and generate the manifests (e.g. CRDs,CRs) with:
$ make manifests
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ kubebuilder create webhook --group batch --version v1 --kind CronJob  --conversion
Writing kustomize manifests for you to edit...
Writing scaffold for you to edit...
api/v1/cronjob_webhook.go
Webhook server has been set up for you.
You need to implement the conversion.Hub and conversion.Convertible interfaces for your CRD types.
Update dependencies:
$ go mod tidy
Running make:
$ make generate
/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
Next: implement your new Webhook and generate the manifests with:
$ make manifests
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ make manifests
/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ kubebuilder version
Version: main.version{KubeBuilderVersion:"v3.10.0-124-g3a3d1d95", KubernetesVendor:"unknown", GitCommit:"3a3d1d9573f5b8fe7252bf49cec6e67ba87c88e7", BuildDate:"2023-06-20T13:45:14Z", GoOs:"darwin", GoArch:"amd64"}
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ make manifests
/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ make generate/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ kustomize build config/crd
-bash: kustomize: command not found
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ kustomize version
-bash: kustomize: command not found
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ make install
/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
test -s /Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/kustomize || GOBIN=/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin GO111MODULE=on go install sigs.k8s.io/kustomize/kustomize/v5@v5.0.1
go: downloading sigs.k8s.io/kustomize/kustomize/v5 v5.0.1
go: downloading sigs.k8s.io/kustomize/api v0.13.2
go: downloading sigs.k8s.io/kustomize/kyaml v0.14.1
go: downloading sigs.k8s.io/kustomize/cmd/config v0.11.1
go: downloading github.com/go-errors/errors v1.4.2
go: downloading github.com/evanphx/json-patch v4.11.0+incompatible
go: downloading k8s.io/kube-openapi v0.0.0-20230109183929-3758b55a6596
/Users/camiladeomacedo/go/src/sigs.k8s.io/testing/bin/kustomize build config/crd | kubectl apply -f -
customresourcedefinition.apiextensions.k8s.io/cronjobs.batch.my.domain created
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ ./bin/kustomize build config/crd
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  annotations:
    controller-gen.kubebuilder.io/version: v0.12.0
  name: cronjobs.batch.my.domain
spec:
  group: batch.my.domain
  names:
    kind: CronJob
    listKind: CronJobList
    plural: cronjobs
    singular: cronjob
  scope: Namespaced
  versions:
  - name: v1
    schema:
      openAPIV3Schema:
        description: CronJob is the Schema for the cronjobs API
        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
          kind:
            description: 'Kind is a string value representing the REST resource this
              object represents. Servers may infer this from the endpoint the client
              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
            type: string
          metadata:
            type: object
          spec:
            description: CronJobSpec defines the desired state of CronJob
            properties:
              foo:
                description: Foo is an example field of CronJob. Edit cronjob_types.go
                  to remove/update
                type: string
            type: object
          status:
            description: CronJobStatus defines the observed state of CronJob
            type: object
        type: object
    served: true
    storage: true
    subresources:
      status: {}
camiladeomacedo@camilas-MacBook-Air ~/go/src/sigs.k8s.io/testing $ 
```

Could you please re-check the kustomize version used, if you uncommented all and etc?

@camilamacedo86 camilamacedo86 added triage/needs-information Indicates an issue needs more information in order to work on it. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. release-blocker labels Jun 20, 2023
@mjlshen
Copy link
Contributor

mjlshen commented Jun 21, 2023

@camilamacedo86 I can reproduce it starting with your steps

kubebuilder init 
kubebuilder create api --group batch --version v1 --kind CronJob
kubebuilder create webhook --group batch --version v1 --kind CronJob  --conversion

and then crucially here, uncommenting all the pieces you pointed out in config/crd/kustomization.yaml and config/default/kustomization.yaml before running make install - or as was originally reported, kustomize build config/crd.

The thing is, uncommenting in config/crd/kustomization.yaml immediately causes (and is the source of) the error. I also started fresh with kustomize v5.0.1:

❯ make kustomize
test -s /Users/mshen/git/mjlshen/testing/bin/kustomize || GOBIN=/Users/mshen/git/mjlshen/testing/bin GO111MODULE=on go install sigs.k8s.io/kustomize/kustomize/v5@v5.0.1

❯ make install     
test -s /Users/mshen/git/mjlshen/testing/bin/controller-gen && /Users/mshen/git/mjlshen/testing/bin/controller-gen --version | grep -q v0.12.0 || \
	GOBIN=/Users/mshen/git/mjlshen/testing/bin go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.12.0
/Users/mshen/git/mjlshen/testing/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/Users/mshen/git/mjlshen/testing/bin/kustomize build config/crd | kubectl apply -f -
Error: invalid Kustomization: json: cannot unmarshal string into Go struct field Kustomization.patches of type types.Patch
error: no objects passed to apply
make: *** [install] Error 1

I think the e2e test we added doesn't try to uncomment these WEBHOOK portions of the codebase, so that's why it wasn't failing. I can work on writing an e2e for this!

@mjlshen
Copy link
Contributor

mjlshen commented Jun 21, 2023

/assign

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/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants