Skip to content

Commit

Permalink
make sure crd-* manifests are YAML before passing to controller-gen
Browse files Browse the repository at this point in the history
The controller-gen tool'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.

This patch uses CUE to parse the YAML files. We could have picked
something like yq (Python) or the Go version of yq, but we don't want
Python dependencies, and the Go version of yq has a poor UX.

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

Signed-off-by: Maël Valais <mael@vls.dev>
  • Loading branch information
maelvls committed Jul 5, 2021
1 parent af0ba9c commit ac327d1
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 2 deletions.
6 changes: 6 additions & 0 deletions hack/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ JQ = "//hack/bin:jq"

CONTROLLER_GEN = "@io_k8s_sigs_controller_tools//cmd/controller-gen"

CUE = "//hack/bin:cue"

# General repo verification targets

py_test(
Expand Down Expand Up @@ -238,10 +240,12 @@ sh_binary(
args = [
"$(location %s)" % GO,
"$(location %s)" % CONTROLLER_GEN,
"$(location %s)" % CUE,
],
data = [
GO,
CONTROLLER_GEN,
CUE,
],
)

Expand All @@ -252,11 +256,13 @@ sh_test(
"$(location :update-crds)",
"$(location %s)" % GO,
"$(location %s)" % CONTROLLER_GEN,
"$(location %s)" % CUE,
],
data = [
":update-crds",
GO,
CONTROLLER_GEN,
CUE,
"@//:all-srcs",
],
)
Expand Down
11 changes: 11 additions & 0 deletions hack/bin/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ genrule(
visibility = ["//visibility:public"],
)

genrule(
name = "fetch_cue",
srcs = select({
":darwin": ["@cue_osx//:file"],
":k8": ["@cue_linux//:file"],
}),
outs = ["cue"],
cmd = "cp $(SRCS) $@",
visibility = ["//visibility:public"],
)

genrule(
name = "io_kubernetes_kube-apiserver",
srcs = select({
Expand Down
37 changes: 35 additions & 2 deletions hack/bin/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def install():
install_oc3()
install_kind()
install_kustomize()
install_cue()

# Install golang.org/x/build as kubernetes/repo-infra requires it for the
# build-tar bazel target.
Expand Down Expand Up @@ -231,15 +232,14 @@ def install_kubectl():
urls = ["https://storage.googleapis.com/kubernetes-release/release/v1.18.0/bin/linux/amd64/kubectl"],
)


# Define rules for different oc versions
def install_oc3():
http_archive(
name = "oc_3_11_linux",
sha256 = "4b0f07428ba854174c58d2e38287e5402964c9a9355f6c359d1242efd0990da3",
urls = ["https://github.com/openshift/origin/releases/download/v3.11.0/openshift-origin-client-tools-v3.11.0-0cbc58b-linux-64bit.tar.gz"],
build_file_content =
"""
"""
filegroup(
name = "file",
srcs = [
Expand All @@ -249,6 +249,7 @@ filegroup(
)
""",
)

## Fetch kind images used during e2e tests
def install_kind():
# install kind binary
Expand All @@ -266,3 +267,35 @@ def install_kind():
urls = ["https://github.com/kubernetes-sigs/kind/releases/download/v0.11.0/kind-linux-amd64"],
)

## Cue is used for linting the CRD YAML that gets generated in ./update-crds.sh.
def install_cue():
# install kind binary
http_archive(
name = "cue_darwin",
sha256 = "24717a72b067a4d8f4243b51832f4a627eaa7e32abc4b9117b0af9aa63ae0332",
urls = ["https://github.com/cuelang/cue/releases/download/v0.4.0/cue_v0.4.0_darwin_amd64.tar.gz"],
build_file_content = """
filegroup(
name = "file",
srcs = [
"cue",
],
visibility = ["//visibility:public"],
)
""",
)

http_archive(
name = "cue_linux",
sha256 = "a118177d9c605b4fc1a61c15a90fddf57a661136c868dbcaa9d2406c95897949",
urls = ["https://github.com/cuelang/cue/releases/download/v0.4.0/cue_v0.4.0_linux_amd64.tar.gz"],
build_file_content = """
filegroup(
name = "file",
srcs = [
"cue",
],
visibility = ["//visibility:public"],
)
""",
)
20 changes: 20 additions & 0 deletions hack/update-crds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,33 @@ else
fi

go=$(realpath "$1")

controllergen="$(realpath "$2")"
cue="$(realpath "$3")"

export PATH=$(dirname "$go"):$PATH

# This script should be run via `bazel run //hack:update-crds`
REPO_ROOT=${BUILD_WORKSPACE_DIRECTORY}
cd "${REPO_ROOT}"

set -xe

# controller-gen silently skips YAML crd-*.yaml files that are not proper
# YAML. As detailed in [1], this issue prevented anyone from updating the
# crd-*.yaml files. To avoid this issue from happening again, we want to
# make sure every CRD file is a valid YAML file, before running
# controller-gen.
#
# We could just use yq (the python CLI) to parse the YAML... But I didn't
# want us to rely on python things. And its Go equivalent, mikefarah/yq,
# has a pretty poor CLI UX. So I just used the excellent CUE tool. ALthough
# CUE is meant for validating schemas, we don't care about schemas here,
# that's why we use a dummy valid-crd.cue.
#
# [1]: https://github.com/jetstack/cert-manager/pull/3989
"$cue" vet hack/valid-crd.cue deploy/crds/*.yaml

"$controllergen" \
schemapatch:manifests=./deploy/crds \
output:dir=./deploy/crds \
Expand Down
2 changes: 2 additions & 0 deletions hack/valid-crd.cue
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// We just validate that our CRDs are valid YAML files, we don't care about
// validating the data itself.

0 comments on commit ac327d1

Please sign in to comment.