diff --git a/.github/workflows/check-crd-compatibility.yaml b/.github/workflows/check-crd-compatibility.yaml index 5aa9f878ea3ab..d5d2ba0ffd62b 100644 --- a/.github/workflows/check-crd-compatibility.yaml +++ b/.github/workflows/check-crd-compatibility.yaml @@ -88,19 +88,17 @@ jobs: - name: Run crdify for platform operator run: | - set -e + set -euo pipefail old_path="$PWD/.old-stackrox/operator/config/crd/bases" new_path="$PWD/operator/config/crd/bases" - crdify="go run -C operator/tools/crdify sigs.k8s.io/crdify --config=config.yaml" - set -x - $crdify "file://${old_path}/platform.stackrox.io_securedclusters.yaml" "file://${new_path}/platform.stackrox.io_securedclusters.yaml" - $crdify "file://${old_path}/platform.stackrox.io_centrals.yaml" "file://${new_path}/platform.stackrox.io_centrals.yaml" + crdify=".github/workflows/scripts/crdify.sh" + $crdify "${old_path}/platform.stackrox.io_securedclusters.yaml" "${new_path}/platform.stackrox.io_securedclusters.yaml" + $crdify "${old_path}/platform.stackrox.io_centrals.yaml" "${new_path}/platform.stackrox.io_centrals.yaml" - name: Run crdify for config operator run: | - set -e + set -euo pipefail old_path="$PWD/.old-stackrox/config-controller/config/crd/bases" new_path="$PWD/config-controller/config/crd/bases" - crdify="go run -C operator/tools/crdify sigs.k8s.io/crdify --config=config.yaml" - set -x - $crdify "file://${old_path}/config.stackrox.io_securitypolicies.yaml" "file://${new_path}/config.stackrox.io_securitypolicies.yaml" + crdify=".github/workflows/scripts/crdify.sh" + $crdify "${old_path}/config.stackrox.io_securitypolicies.yaml" "${new_path}/config.stackrox.io_securitypolicies.yaml" diff --git a/.github/workflows/scripts/crdify.sh b/.github/workflows/scripts/crdify.sh new file mode 100755 index 0000000000000..37e9ce64b545f --- /dev/null +++ b/.github/workflows/scripts/crdify.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# Run crdify on copies of CRDs after stripping out known false positives. +# Usage: +# crdify.sh [crdify flags] + +set -euo pipefail + +ROOT_DIR="$(dirname "${BASH_SOURCE[0]}")/../../.." +readonly ROOT_DIR + +if [[ $# -lt 2 ]]; then + echo "usage: $0 [crdify flags]" >&2 + exit 2 +fi + +old_crd="${1:?missing required argument 'old-crd'}" +new_crd="${2:?missing required argument 'new-crd'}" +shift +shift + +if [[ ! -f "$old_crd" ]]; then + echo "::error::CRD file not found: $old_crd" >&2 + exit 1 +fi + +if [[ ! -f "$new_crd" ]]; then + echo "::error::CRD file not found: $new_crd" >&2 + exit 1 +fi + + +# The addition of this particular validation rule, even though considered a breaking change by definition +# (https://github.com/kubernetes/community/blob/main/contributors/devel/sig-architecture/api_changes.md#backward-compatibility-gotchas) +# is fine in practice, since the `excludeByKind` branch is being added in the same release cycle as the rule itself, +# meaning no production CRS could have existed that violated the validation. +filter_spec_exclusions_one_of() { + local input="$1" + local output="$2" + + local message='at most one of the fields in [deployment excludeByKind] may be set' + local rule='(has(self.deployment)?1:0)+(has(self.excludeByKind)?1:0) <= 1' + + # Remove matching rule, and the validations list if empty afterwards. + message="${message}" rule="${rule}" \ + go run -C "${ROOT_DIR}/operator/tools/yq" github.com/mikefarah/yq/v4 eval ' + del( + .spec.versions[].schema.openAPIV3Schema.properties.spec.properties.exclusions.items."x-kubernetes-validations"[] + | select(.message == strenv(message) and .rule == strenv(rule)) + ) + | del( + .spec.versions[].schema.openAPIV3Schema.properties.spec.properties.exclusions.items."x-kubernetes-validations" + | select(length == 0) + ) + ' "$input" > "$output" +} + + +crd_basename="$(basename "${old_crd}")" +case "$crd_basename" in +config.stackrox.io_securitypolicies.yaml) + filtered_dir="$(mktemp -d)" + echo >&2 "Filtering known false positives from ${old_crd} and ${new_crd}" + filter_spec_exclusions_one_of "${old_crd}" "${filtered_dir}/old-${crd_basename}" + filter_spec_exclusions_one_of "${new_crd}" "${filtered_dir}/new-${crd_basename}" + old_crd="${filtered_dir}/old-${crd_basename}" + new_crd="${filtered_dir}/new-${crd_basename}" + ;; +*) + echo >&2 "No filtering rules for ${old_crd}, running crdify on original CRDs." + ;; +esac + +set -x +go run -C "${ROOT_DIR}/operator/tools/crdify" sigs.k8s.io/crdify --config=config.yaml "$@" \ + "file://${old_crd}" "file://${new_crd}" diff --git a/config-controller/api/v1alpha1/policy_types.go b/config-controller/api/v1alpha1/policy_types.go index 6c777f83d3e10..3563ae52b0f4c 100644 --- a/config-controller/api/v1alpha1/policy_types.go +++ b/config-controller/api/v1alpha1/policy_types.go @@ -90,10 +90,11 @@ type SecurityPolicySpec struct { MitreVectorsLocked bool `json:"mitreVectorsLocked,omitempty"` } +// +kubebuilder:validation:AtMostOneOf=deployment;excludeByKind type Exclusion struct { Name string `json:"name,omitempty"` // +optional - Deployment Deployment `json:"deployment,omitempty"` + Deployment Deployment `json:"deployment,omitzero"` Image Image `json:"image,omitempty"` // +optional // ExcludeByKind excludes all workloads of the selected Kubernetes kinds. diff --git a/config-controller/api/v1alpha1/policy_types_test.go b/config-controller/api/v1alpha1/policy_types_test.go index 0f62a46355a68..2e95d9bb1e5a2 100644 --- a/config-controller/api/v1alpha1/policy_types_test.go +++ b/config-controller/api/v1alpha1/policy_types_test.go @@ -1,6 +1,7 @@ package v1alpha1 import ( + "encoding/json" "testing" "github.com/stackrox/rox/generated/storage" @@ -358,3 +359,21 @@ func TestToProtobufExcludeByKind(t *testing.T) { assert.EqualError(t, err, `excludeByKind contains unknown workload kind "CRONJOB"`) }) } + +func TestExclusionJSONOmitsZeroDeployment(t *testing.T) { + kindOnly, err := json.Marshal(Exclusion{ + Name: "skip jobs", + ExcludeByKind: &ExcludeByKind{Kinds: []WorkloadKind{"JOB"}}, + }) + assert.NoError(t, err) + assert.NotContains(t, string(kindOnly), `"deployment"`) + assert.Contains(t, string(kindOnly), `"excludeByKind"`) + + withDeployment, err := json.Marshal(Exclusion{ + Name: "skip web", + Deployment: Deployment{Name: "web"}, + }) + assert.NoError(t, err) + assert.Contains(t, string(withDeployment), `"deployment"`) + assert.Contains(t, string(withDeployment), `"web"`) +} diff --git a/config-controller/config/crd/bases/config.stackrox.io_securitypolicies.yaml b/config-controller/config/crd/bases/config.stackrox.io_securitypolicies.yaml index 6b88f55eb6644..d3a1254eabdb2 100644 --- a/config-controller/config/crd/bases/config.stackrox.io_securitypolicies.yaml +++ b/config-controller/config/crd/bases/config.stackrox.io_securitypolicies.yaml @@ -170,6 +170,11 @@ spec: name: type: string type: object + x-kubernetes-validations: + - message: at most one of the fields in [deployment excludeByKind] + may be set + rule: (has(self.deployment)?1:0)+(has(self.excludeByKind)?1:0) + <= 1 type: array isDefault: description: IsDefault is unused diff --git a/image/templates/helm/stackrox-central/internal/config.stackrox.io_securitypolicies.yaml b/image/templates/helm/stackrox-central/internal/config.stackrox.io_securitypolicies.yaml index 6b88f55eb6644..d3a1254eabdb2 100644 --- a/image/templates/helm/stackrox-central/internal/config.stackrox.io_securitypolicies.yaml +++ b/image/templates/helm/stackrox-central/internal/config.stackrox.io_securitypolicies.yaml @@ -170,6 +170,11 @@ spec: name: type: string type: object + x-kubernetes-validations: + - message: at most one of the fields in [deployment excludeByKind] + may be set + rule: (has(self.deployment)?1:0)+(has(self.excludeByKind)?1:0) + <= 1 type: array isDefault: description: IsDefault is unused diff --git a/operator/bundle/manifests/config.stackrox.io_securitypolicies.yaml b/operator/bundle/manifests/config.stackrox.io_securitypolicies.yaml index 39f6730c36970..dadcf9abe6338 100644 --- a/operator/bundle/manifests/config.stackrox.io_securitypolicies.yaml +++ b/operator/bundle/manifests/config.stackrox.io_securitypolicies.yaml @@ -171,6 +171,11 @@ spec: name: type: string type: object + x-kubernetes-validations: + - message: at most one of the fields in [deployment excludeByKind] + may be set + rule: (has(self.deployment)?1:0)+(has(self.excludeByKind)?1:0) + <= 1 type: array isDefault: description: IsDefault is unused