make sure crd-* manifests are YAML before passing to controller-gen

[maelvls]

As described in #3989, controller-gen's schemapatch option silently skips CRDs files that are not valid YAML. We often forget that, and end up merging a CRD file that we silently won't be able to update using ./hack/update-crds.sh like what happened in #3932.

This patch uses the excellent cue to parse the YAML files. I could have picked something like yq (Python) or its Go port, mikefarah/yq, but we don't want Python dependencies, and the Go port mikefarah/yq isn't great (e.g., does not show the file name when a parsing error happens...).

Here is what you can expect when one of the CRDs isn't a proper YAML file:

  % ./hack/update-crds.sh
  /home/mvalais/code/cert-manager/deploy/crds/crd-orders.yaml:14: did not find expected key

Note to the reviewer:

I don't know whether using CUE is really a good idea... I don't know how to tell Bazel to download CUE either, so I just defaulted to using Go 1.16's go install @version syntax.

/kind cleanup
/release-note-none

[jetstack-bot]

@maelvls: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maelvls

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [maelvls]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wallrj]

Looks like some tests failed, and I think you will need to figure out how to get Bazel to install the cue tool. See https://github.com/jetstack/cert-manager/blob/3bb830c2abb40742466452b301ab9f1b64f47eda/hack/BUILD.bazel#L235-L262

Also might be worth considering kubeval: https://kubeval.instrumenta.dev/

$ kubeval deploy/crds/*.yaml
ERR  - Failed to decode YAML from deploy/crds/crd-certificaterequests.yaml: error converting YAML to JSON: yaml: line 32: did not find expected node content
ERR  - Failed to decode YAML from deploy/crds/crd-certificates.yaml: error converting YAML to JSON: yaml: line 32: did not find expected node content
ERR  - Failed to decode YAML from deploy/crds/crd-challenges.yaml: error converting YAML to JSON: yaml: line 30: did not find expected node content
ERR  - Failed to decode YAML from deploy/crds/crd-clusterissuers.yaml: error converting YAML to JSON: yaml: line 29: did not find expected node content
ERR  - Failed to decode YAML from deploy/crds/crd-issuers.yaml: error converting YAML to JSON: yaml: line 29: did not find expected node content
ERR  - Failed to decode YAML from deploy/crds/crd-orders.yaml: error converting YAML to JSON: yaml: line 30: did not find expected node content

[wallrj]

I went looking to see if there's a kubectl validation command but it seems not: kubernetes/kubernetes#64830

I tried dry-run=client, but it still requires a connection to an API server:

$ kubectl apply --dry-run=client -f deploy/crds/crd-*
The connection to the server localhost:8080 was refused - did you specify the right host or port?

[munnerz]

I don't know whether using CUE is really a good idea... I don't know how to tell Bazel to download CUE either, so I just defaulted to using Go 1.16's go install @Version syntax.

Happy to help with running through how to wire up Bazel for this! You should be able to take a look at controller-gen as an example of depending on a binary dependency of another project too 😄

[maelvls]

I'm not quite familiar with Bazel 😅 I'll look into it

[maelvls]

Phew... After a lot of struggling, I went back to just go install 😞

I just can't be bothered struggling with Bazel when we can just go install with Go 1.16. We still keep the sandboxing by setting GOPATH to Bazel's $PWD:

# Tools installed with "go install" below are sandboxed by Bazel.
export GOPATH=$PWD
export PATH=$(dirname "$go"):$GOPATH/bin:$PATH

# Only installs the first time you run ./hack/update-crds.sh.
go install cuelang.org/go/cmd/cue@v0.4.0-beta.1
go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.2.9

cue vet hack/valid-crd.cue deploy/crds/*.yaml
controller-gen schemapatch:manifests=./deploy/crds output:dir=./deploy/crds paths=./pkg/apis/...

The only downside with this approach is that go install does a quick network call every time we run ./hack/update-crds.sh, but the installation only happens once. This overhead seems negligible compared to the cost of running this script inside Bazel.

I know I should be doing it "properly" by using sh_binary and go_binary but this go install solution seems so much more elegant... 😅

cc @wallrj @irbekrm

[irbekrm]

Do we run this script in CI? I think the images might not have go as we use Bazel to download go.

I'd also be happy to look at the Bazel bits.

[maelvls]

@irbekrm I removed the go install lines and added bazel rules.

[maelvls]

apiserver.go:64: failed to start control plane: unable to start control plane itself:
failed to start the controlplane. retried 5 times: timeout waiting for process etcd
to start successfully (it may have failed to start, or stopped unexpectedly before
becoming ready)

/test pull-cert-manager-bazel

[irbekrm]

Looks great to me, glad we'll have this check.

I've only added a nit, happy to lgtm otherwise.

Note- I did briefly look at whether we can use any of the existing tools for this, but looks like we cannot. I guess it would be nice if we could use a single yaml validation/interpolation tool for all yaml templating/validation etc, but that doesn't seem possible.

[irbekrm]

Nit: whitespace

[jetstack-bot]

@maelvls: PR needs rebase.

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.

[jetstack-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

[jetstack-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

[jetstack-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

[jetstack-bot]

@jetstack-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

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.