Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and Cust
generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."

.PHONY: validate-rbac
validate-rbac: ## Validate that RBAC resourceNames in kubebuilder annotations match actual resource names.
./hack/validate-rbac-resourcenames.sh

.PHONY: fmt
fmt: ## Run go fmt against code.
go fmt ./...
Expand Down Expand Up @@ -280,13 +284,8 @@ OPERATOR_SDK ?= $(LOCALBIN)/operator-sdk
operator-sdk: ## Download operator-sdk locally if necessary.
ifeq (,$(wildcard $(OPERATOR_SDK)))
ifeq (, $(shell which operator-sdk 2>/dev/null))
@{ \
set -e ;\
mkdir -p $(dir $(OPERATOR_SDK)) ;\
OS=$(shell go env GOOS) && ARCH=$(shell go env GOARCH) && \
curl -sSLo $(OPERATOR_SDK) https://github.com/operator-framework/operator-sdk/releases/download/$(OPERATOR_SDK_VERSION)/operator-sdk_$${OS}_$${ARCH} ;\
chmod +x $(OPERATOR_SDK) ;\
}
mkdir -p $(dir $(OPERATOR_SDK))
hack/operator-sdk.sh $(OPERATOR_SDK) $(OPERATOR_SDK_VERSION)
else
OPERATOR_SDK = $(shell which operator-sdk)
endif
Expand Down Expand Up @@ -370,7 +369,7 @@ catalog-push: ## Push a catalog image.

## verify the changes are working as expected.
.PHONY: verify
verify: vet fmt golangci-lint verify-bindata verify-bindata-assets verify-generated
verify: vet fmt golangci-lint verify-bindata verify-bindata-assets verify-generated validate-rbac

## update the relevant data based on new changes.
.PHONY: update
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the resourceNames might not help as expected with ClusterRole, because of limitations with create, list, and watch verbs.

xref: https://kubernetes.io/docs/reference/access-authn-authz/rbac/

Note:
You cannot restrict create or deletecollection requests by their resource name. For create, this limitation is because the name of the new object may not be known at authorization time. If you restrict list or watch by resourceName, clients must include a metadata.name field selector in their list or watch request that matches the specified resourceName in order to be authorized. For example, kubectl get configmaps --field-selector=metadata.name=my-configmap

Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ metadata:
categories: Security
console.openshift.io/disable-operand-delete: "true"
containerImage: openshift.io/external-secrets-operator:latest
createdAt: "2025-08-18T11:50:12Z"
createdAt: "2025-08-21T05:49:03Z"
features.operators.openshift.io/cnf: "false"
features.operators.openshift.io/cni: "false"
features.operators.openshift.io/csi: "false"
Expand Down Expand Up @@ -379,19 +379,25 @@ spec:
resources:
- validatingwebhookconfigurations
verbs:
- create
- get
- list
- watch
Comment on lines 382 to +384
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add global “create” for ValidatingWebhookConfigurations.

Otherwise the operator cannot create the webhook objects; resourceNames cannot scope create.

          resources:
          - validatingwebhookconfigurations
          verbs:
+         - create
          - get
          - list
          - watch
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- get
- list
- watch
resources:
- validatingwebhookconfigurations
verbs:
- create
- get
- list
- watch
🤖 Prompt for AI Agents
In bundle/manifests/external-secrets-operator.clusterserviceversion.yaml around
lines 382-384, the RBAC rule for ValidatingWebhookConfigurations only includes
get/list/watch which prevents the operator from creating webhook objects; add
the "create" verb to that rule (ensure the rule targets the
admissionregistration.k8s.io API group and the validatingwebhookconfigurations
resource) so the operator can create webhook resources cluster-wide.

- apiGroups:
- admissionregistration.k8s.io
resourceNames:
- externalsecret-validate
- secretstore-validate
resources:
- validatingwebhookconfigurations
verbs:
- create
- patch
- update
Comment on lines +386 to 395
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove “create” from the resourceNames-scoped webhook rule.

This does not authorize create and gives a false sense of scoping.

          verbs:
-         - create
          - patch
          - update
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- admissionregistration.k8s.io
resourceNames:
- externalsecret-validate
- secretstore-validate
resources:
- validatingwebhookconfigurations
verbs:
- create
- patch
- update
- admissionregistration.k8s.io
resourceNames:
- externalsecret-validate
- secretstore-validate
resources:
- validatingwebhookconfigurations
verbs:
- patch
- update
🤖 Prompt for AI Agents
In bundle/manifests/external-secrets-operator.clusterserviceversion.yaml around
lines 386 to 395, the webhook rule that is scoped to specific resourceNames
incorrectly includes the "create" verb which does not actually authorize create
for resourceNames-scoped rules; remove the "create" entry from the verbs list so
only valid verbs (e.g., patch, update) remain for that rule, and verify the rule
now correctly reflects the intended scoping.

- watch
- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- create
- delete
- get
- list
- patch
Expand All @@ -402,11 +408,21 @@ spec:
resources:
- deployments
verbs:
- list
- watch
- apiGroups:
- apps
resourceNames:
- bitwarden-sdk-server
- external-secrets
- external-secrets-cert-controller
- external-secrets-webhook
resources:
- deployments
verbs:
- create
- get
- list
- update
- watch
- apiGroups:
- cert-manager.io
resources:
Expand Down
28 changes: 22 additions & 6 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,25 @@ rules:
resources:
- validatingwebhookconfigurations
verbs:
- create
- get
- list
- watch
- apiGroups:
Comment on lines 44 to +47
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Create on ValidatingWebhookConfigurations must be global (resourceNames cannot scope create).

A rule with resourceNames never matches create (name unknown at auth time). Without a global create on validatingwebhookconfigurations, the operator won’t be able to create the webhooks.

Apply:

   resources:
   - validatingwebhookconfigurations
   verbs:
+  - create
   - get
   - list
   - watch
🤖 Prompt for AI Agents
config/rbac/role.yaml around lines 44-47: the current rule scopes create via
resourceNames which never matches CREATE (name unknown), so add a global create
for ValidatingWebhookConfigurations; update the rule so the verbs array includes
"create" at the top-level (not scoped by resourceNames) and ensure the rule
targets the validatingwebhookconfigurations resource in the
admissionregistration.k8s.io API group, or split into two rules where one global
rule grants create for validatingwebhookconfigurations and the other rule keeps
the resourceNames-scoped verbs for get/list/watch if needed.

- admissionregistration.k8s.io
resourceNames:
- externalsecret-validate
- secretstore-validate
resources:
- validatingwebhookconfigurations
verbs:
- create
- patch
Comment on lines +48 to 56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove “create” from the resourceNames-scoped webhook rule.

Create cannot be name-scoped; this rule won’t authorize create anyway and is misleading.

   verbs:
-  - create
   - patch
   - update

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In config/rbac/role.yaml around lines 48-56, the webhook RBAC rule that is
scoped to resourceNames incorrectly includes the "create" verb (name-scoped
rules cannot authorize create); remove the "create" entry from the verbs list
for the resourceNames-scoped validatingwebhookconfigurations rule so only valid
verbs (e.g., patch) remain.

- update
- watch
- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- create
- delete
- get
- list
- patch
Expand All @@ -64,11 +70,21 @@ rules:
resources:
- deployments
verbs:
- list
- watch
- apiGroups:
- apps
resourceNames:
- bitwarden-sdk-server
- external-secrets
- external-secrets-cert-controller
- external-secrets-webhook
resources:
- deployments
verbs:
- create
- get
- list
- update
- watch
- apiGroups:
- cert-manager.io
resources:
Expand Down
37 changes: 37 additions & 0 deletions hack/operator-sdk.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/bin/sh

set -e

OUTPUT_PATH=${1:-./bin/operator-sdk}
VERSION=${2:-"v1.39.0"}

GOOS=$(go env GOOS)
GOARCH=$(go env GOARCH)
BIN="operator-sdk"
BIN_ARCH="${BIN}_${GOOS}_${GOARCH}"
OPERATOR_SDK_DL_URL="https://github.com/operator-framework/operator-sdk/releases/download/${VERSION}"

if [[ "$GOOS" != "linux" && "$GOOS" != "darwin" ]]; then
echo "Unsupported OS $GOOS"
exit 1
fi

if [[ "$GOARCH" != "amd64" && "$GOARCH" != "arm64" && "$GOARCH" != "ppc64le" && "$GOARCH" != "s390x" ]]; then
echo "Unsupported architecture $GOARCH"
exit 1
fi
Comment on lines +14 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix bashisms under /bin/sh (will fail on dash).

The script uses [[ … ]] and &> which are bash-only. With the current shebang (/bin/sh), this breaks on many systems.

Apply:

-if [[ "$GOOS" != "linux" && "$GOOS" != "darwin" ]]; then
-  echo "Unsupported OS $GOOS"
-  exit 1
-fi
+case "$GOOS" in
+  linux|darwin) ;;
+  *) echo "Unsupported OS $GOOS"; exit 1 ;;
+esac
-
-if [[ "$GOARCH" != "amd64" && "$GOARCH" != "arm64" && "$GOARCH" != "ppc64le" && "$GOARCH" != "s390x" ]]; then
-  echo "Unsupported architecture $GOARCH"
-  exit 1
-fi
+case "$GOOS/$GOARCH" in
+  linux/amd64|linux/arm64|linux/ppc64le|linux/s390x|darwin/amd64|darwin/arm64) ;;
+  *) echo "Unsupported architecture $GOARCH for $GOOS"; exit 1 ;;
+esac
-
-command -v curl &> /dev/null || { echo "can't find curl command" && exit 1; }
+command -v curl >/dev/null 2>&1 || { echo "can't find curl command" >&2; exit 1; }

Also applies to: 24-24

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 14-14: In POSIX sh, [[ ]] is undefined.

(SC3010)


[warning] 19-19: In POSIX sh, [[ ]] is undefined.

(SC3010)

🤖 Prompt for AI Agents
In hack/operator-sdk.sh around lines 14-22 (and also line 24) replace bash-only
constructs so the script is POSIX /bin/sh compatible: change the [[ ... ]] tests
to POSIX [ ... ] (or use a case statement) with proper spacing and use -o or
separate tests combined with &&/|| as needed, and replace any &> redirects with
the POSIX equivalent >file 2>&1 (or >/dev/stderr as appropriate); ensure the
shebang stays /bin/sh and update both the OS and GOARCH checks (and the
occurrence at line 24) accordingly.


command -v curl &> /dev/null || { echo "can't find curl command" && exit 1; }

TEMPDIR=$(mktemp -d)
BIN_PATH="${TEMPDIR}/${BIN_ARCH}"

echo "> downloading binary"
curl --location -o "${BIN_PATH}" "${OPERATOR_SDK_DL_URL}/operator-sdk_${GOOS}_${GOARCH}"

echo "> installing binary"
mv "${BIN_PATH}" "${OUTPUT_PATH}"
chmod +x "${OUTPUT_PATH}"
rm -rf "${TEMPDIR}"
Comment on lines +26 to +35
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden download/install and guarantee cleanup.

Add trap for tempdir cleanup, fail on HTTP errors, ensure destination dir exists, and use install(1) atomically.

 TEMPDIR=$(mktemp -d)
 BIN_PATH="${TEMPDIR}/${BIN_ARCH}"
+trap 'rm -rf "${TEMPDIR}"' EXIT
 
 echo "> downloading binary"
-curl --location -o "${BIN_PATH}" "${OPERATOR_SDK_DL_URL}/operator-sdk_${GOOS}_${GOARCH}"
+curl -fSL -o "${BIN_PATH}" "${OPERATOR_SDK_DL_URL}/operator-sdk_${GOOS}_${GOARCH}"
 
 echo "> installing binary"
-mv "${BIN_PATH}" "${OUTPUT_PATH}"
-chmod +x "${OUTPUT_PATH}"
-rm -rf "${TEMPDIR}"
+mkdir -p "$(dirname "${OUTPUT_PATH}")"
+install -m 0755 "${BIN_PATH}" "${OUTPUT_PATH}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
TEMPDIR=$(mktemp -d)
BIN_PATH="${TEMPDIR}/${BIN_ARCH}"
echo "> downloading binary"
curl --location -o "${BIN_PATH}" "${OPERATOR_SDK_DL_URL}/operator-sdk_${GOOS}_${GOARCH}"
echo "> installing binary"
mv "${BIN_PATH}" "${OUTPUT_PATH}"
chmod +x "${OUTPUT_PATH}"
rm -rf "${TEMPDIR}"
TEMPDIR=$(mktemp -d)
BIN_PATH="${TEMPDIR}/${BIN_ARCH}"
trap 'rm -rf "${TEMPDIR}"' EXIT
echo "> downloading binary"
curl -fSL -o "${BIN_PATH}" "${OPERATOR_SDK_DL_URL}/operator-sdk_${GOOS}_${GOARCH}"
echo "> installing binary"
mkdir -p "$(dirname "${OUTPUT_PATH}")"
install -m 0755 "${BIN_PATH}" "${OUTPUT_PATH}"
🤖 Prompt for AI Agents
In hack/operator-sdk.sh around lines 26 to 35, make the download/install more
robust: create the temp dir with mktemp -d as now but register a trap 'trap "rm
-rf \"$TEMPDIR\"" EXIT' immediately after to guarantee cleanup; make curl fail
on HTTP errors and show errors by using --fail --show-error --location --output
(or -fSLo) so the script exits on non-2xx responses; ensure the destination
directory exists with 'mkdir -p "$(dirname "$OUTPUT_PATH")"' before installing;
replace mv+chmod with an atomic install(1) call like 'install -m 0755
"$BIN_PATH" "$OUTPUT_PATH"' (which sets executable bit and moves atomically) and
remove the explicit chmod and mv; also run the script with strict failure flags
(set -euo pipefail) if not already present to abort on errors.


echo "> operator-sdk binary available at ${OUTPUT_PATH}"
156 changes: 156 additions & 0 deletions hack/validate-rbac-resourcenames.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
#!/bin/bash

# validate-rbac-resourcenames.sh
# This script validates that the resourceNames in kubebuilder RBAC annotations
# match the actual resource names defined in the assets.

set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PROJECT_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)"

# extract resourceNames from kubebuilder annotations
extract_kubebuilder_resourcenames() {
local resource_type="$1"
grep -E "^\s*//\s*\+kubebuilder:rbac:.*resources=${resource_type}.*resourceNames=" \
"${PROJECT_ROOT}/pkg/controller/external_secrets/controller.go" | \
sed -E 's/.*resourceNames=([^,]*).*/\1/' | \
tr ';' '\n' | sort -u
Comment on lines +15 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Regex uses ‘\s’ which grep -E doesn’t support; extractor will return empty.

Use POSIX classes.

-    grep -E "^\s*//\s*\+kubebuilder:rbac:.*resources=${resource_type}.*resourceNames=" \
+    grep -E "^[[:space:]]*//[[:space:]]*\+kubebuilder:rbac:.*resources=${resource_type}.*resourceNames=" \
         "${PROJECT_ROOT}/pkg/controller/external_secrets/controller.go" | \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
grep -E "^\s*//\s*\+kubebuilder:rbac:.*resources=${resource_type}.*resourceNames=" \
"${PROJECT_ROOT}/pkg/controller/external_secrets/controller.go" | \
sed -E 's/.*resourceNames=([^,]*).*/\1/' | \
tr ';' '\n' | sort -u
grep -E "^[[:space:]]*//[[:space:]]*\+kubebuilder:rbac:.*resources=${resource_type}.*resourceNames=" \
"${PROJECT_ROOT}/pkg/controller/external_secrets/controller.go" | \
sed -E 's/.*resourceNames=([^,]*).*/\1/' | \
tr ';' '\n' | sort -u
🤖 Prompt for AI Agents
In hack/validate-rbac-resourcenames.sh around lines 15 to 18, the grep -E
pattern uses \s (not supported by grep -E) causing no matches; replace
backslash-s with the POSIX character class [:space:] (e.g. use [[:space:]]* in
place of \s*) so the regex correctly matches whitespace, leaving the rest of the
pipeline (sed/tr/ sort) unchanged.

}

# extract actual resource names from assets
extract_asset_names() {
local pattern="$1"
find "${PROJECT_ROOT}/bindata/external-secrets/resources" "${PROJECT_ROOT}/bindata/external-secrets" -name "*.yml" -exec grep -l "kind: ${pattern}" {} \; 2>/dev/null | \
xargs grep -h "^ name:" | \
awk '{print $2}' | sort -u
}
Comment on lines +24 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Handle .yaml files and filenames with spaces; avoid SC2038.

Use -exec … + and include both extensions; match indentation-agnostic “name:”.

-extract_asset_names() {
+extract_asset_names() {
     local pattern="$1"
-    find "${PROJECT_ROOT}/bindata/external-secrets/resources" "${PROJECT_ROOT}/bindata/external-secrets" -name "*.yml" -exec grep -l "kind: ${pattern}" {} \; 2>/dev/null | \
-        xargs grep -h "^  name:" | \
-        awk '{print $2}' | sort -u
+    find "${PROJECT_ROOT}/bindata/external-secrets/resources" "${PROJECT_ROOT}/bindata/external-secrets" \
+        -type f \( -name "*.yml" -o -name "*.yaml" \) -exec grep -l "kind: ${pattern}" {} + 2>/dev/null | \
+        xargs -r grep -hE '^[[:space:]]{2}name:' | \
+        awk '{print $2}' | sort -u
 }

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 24-24: Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames.

(SC2038)

🤖 Prompt for AI Agents
In hack/validate-rbac-resourcenames.sh around lines 24-27, the find/grep
pipeline only matches .yml, breaks on filenames with spaces, and is
indentation-sensitive; update the find invocation to include both "*.yml" and
"*.yaml" (using -name A -o -name B), use -exec ... + or -print0 combined with
xargs -0 to safely handle spaces (avoid plain xargs to address SC2038), and
change the final grep to match "name:" with optional leading whitespace (e.g.,
use a regex like '^\s*name:') so it is indentation-agnostic before extracting
the resource name.


# extract CRD names from assets
extract_crd_names() {
# Get CRDs from config/crd/bases
local crd_names=""
if [[ -d "${PROJECT_ROOT}/config/crd/bases" ]]; then
crd_names=$(find "${PROJECT_ROOT}/config/crd/bases" -name "*.yml" -o -name "*.yaml" 2>/dev/null | \
xargs grep -l "kind: CustomResourceDefinition" 2>/dev/null | \
xargs grep -h "^ name:" 2>/dev/null | \
awk '{print $2}')
Comment on lines +34 to +37
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Same robustness fix for CRD discovery; avoid xargs on unsanitized filenames.

-        crd_names=$(find "${PROJECT_ROOT}/config/crd/bases" -name "*.yml" -o -name "*.yaml" 2>/dev/null | \
-            xargs grep -l "kind: CustomResourceDefinition" 2>/dev/null | \
-            xargs grep -h "^  name:" 2>/dev/null | \
-            awk '{print $2}')
+        crd_names=$(find "${PROJECT_ROOT}/config/crd/bases" -type f \( -name "*.yml" -o -name "*.yaml" \) \
+            -exec grep -l "kind: CustomResourceDefinition" {} + 2>/dev/null | \
+            xargs -r grep -hE '^[[:space:]]{2}name:' 2>/dev/null | \
+            awk '{print $2}')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
crd_names=$(find "${PROJECT_ROOT}/config/crd/bases" -name "*.yml" -o -name "*.yaml" 2>/dev/null | \
xargs grep -l "kind: CustomResourceDefinition" 2>/dev/null | \
xargs grep -h "^ name:" 2>/dev/null | \
awk '{print $2}')
crd_names=$(find "${PROJECT_ROOT}/config/crd/bases" -type f \( -name "*.yml" -o -name "*.yaml" \) \
-exec grep -l "kind: CustomResourceDefinition" {} + 2>/dev/null | \
xargs -r grep -hE '^[[:space:]]{2}name:' 2>/dev/null | \
awk '{print $2}')
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 34-34: Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames.

(SC2038)

🤖 Prompt for AI Agents
In hack/validate-rbac-resourcenames.sh around lines 34 to 37, the CRD discovery
pipeline uses xargs on unsanitized filenames which breaks on spaces/special
chars; change the pipeline to use null-delimited paths (find ... -print0) and
xargs -0 for the grep steps or replace the pipeline with a safe while-read loop
that reads -d '' from find -print0 and runs grep -h "^  name:" on each file,
collecting the second field as before so filenames with spaces/newlines are
handled correctly.

fi
echo "${crd_names}" | grep -v "^$" | sort -u
}

# compare kubebuilder resourceNames with actual resources
compare_resources() {
local resource_display_name="$1"
local kubebuilder_resources="$2"
local actual_resources="$3"

echo "Kubebuilder resourceNames:"
echo "${kubebuilder_resources}" | sed 's/^/ - /'
echo
echo "Actual ${resource_display_name} names:"
echo "${actual_resources}" | sed 's/^/ - /'
echo

# Compare - find missing and extra resources
local missing_in_kb
missing_in_kb=$(comm -23 <(echo "${actual_resources}") <(echo "${kubebuilder_resources}"))

local extra_in_kb
extra_in_kb=$(comm -13 <(echo "${actual_resources}") <(echo "${kubebuilder_resources}"))

local has_errors=false

if [[ -n "${missing_in_kb}" ]]; then
echo "Missing in kubebuilder annotations:"
echo "${missing_in_kb}" | sed 's/^/ /'
has_errors=true
fi

if [[ -n "${extra_in_kb}" ]]; then
echo "Extra in kubebuilder annotations (might be outdated):"
echo "${extra_in_kb}" | sed 's/^/ /'
fi

if [[ "$has_errors" == "false" ]]; then
echo "${resource_display_name} validation passed"
return 0
else
return 1
fi
}

# Generic validation function
validate_resource_type() {
local resource_display_name="$1"
local kubebuilder_resource_type="$2"
local asset_kind="$3"
local extract_func="$4"

echo "Validating ${resource_display_name}..."

# Extract from kubebuilder annotations
local kb_resources
kb_resources=$(extract_kubebuilder_resourcenames "${kubebuilder_resource_type}" || echo "")

# Extract from assets using the specified function
local actual_resources
if [[ "$extract_func" == "extract_crd_names" ]]; then
actual_resources=$(extract_crd_names)
else
actual_resources=$(extract_asset_names "${asset_kind}")
fi

# Compare and report
compare_resources "${resource_display_name}" "${kb_resources}" "${actual_resources}"
}

validate_deployments() {
validate_resource_type "Deployments" "deployments" "Deployment" "extract_asset_names"
}

validate_webhooks() {
validate_resource_type "ValidatingWebhookConfigurations" "validatingwebhookconfigurations" "ValidatingWebhookConfiguration" "extract_asset_names"
}

validate_crds() {
validate_resource_type "CustomResourceDefinitions" "customresourcedefinitions" "" "extract_crd_names"
}

validate_roles() {
validate_resource_type "Roles" "roles" "Role" "extract_asset_names"
}

validate_rolebindings() {
validate_resource_type "RoleBindings" "rolebindings" "RoleBinding" "extract_asset_names"
}

main() {
local exit_code=0

echo "Validating RBAC resourceNames consistency for external-secrets-operator"
echo "=================================================================================="

validate_deployments || exit_code=1
echo
validate_webhooks || exit_code=1
echo
validate_crds || exit_code=1
echo
validate_roles || exit_code=1
echo
validate_rolebindings || exit_code=1
echo

if [[ $exit_code -eq 0 ]]; then
echo "All RBAC resourceNames validations passed!"
echo "The kubebuilder annotations are consistent with the actual resources."
else
echo "RBAC validation failed!"
echo "Please update the kubebuilder annotations in pkg/controller/external_secrets/controller.go"
fi

return $exit_code
}

main "$@"
8 changes: 5 additions & 3 deletions pkg/controller/external_secrets/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,16 +97,18 @@ type Reconciler struct {
// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;watch;create;update;patch

// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings;clusterroles;clusterrolebindings,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;list;watch;create;update;patch
// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;list;watch
// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=create;update;patch,resourceNames=externalsecret-validate;secretstore-validate
// +kubebuilder:rbac:groups="",resources=events;secrets;services;serviceaccounts,verbs=get;list;watch;create;update;delete;patch
Comment on lines +100 to 102
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Webhook RBAC: add global create; remove create from name-scoped rule.

Create cannot be authorized with resourceNames. Without a global create, webhook creation will fail and escalate checks will also fail.

-// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;list;watch
-// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=create;update;patch,resourceNames=externalsecret-validate;secretstore-validate
+// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=create;get;list;watch
+// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=update;patch,resourceNames=externalsecret-validate;secretstore-validate
🤖 Prompt for AI Agents
In pkg/controller/external_secrets/controller.go around lines 100 to 102, the
RBAC rules declare create on validatingwebhookconfigurations only with
resourceNames which is invalid because create cannot be scoped by resourceNames;
add a separate global rule granting create on validatingwebhookconfigurations
(verbs=create) and remove create from the name-scoped RBAC line so the webhook
can be created successfully while keeping name-scoped update/patch as needed.

// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=list;watch
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;create;update,resourceNames=external-secrets;external-secrets-cert-controller;external-secrets-webhook;bitwarden-sdk-server
// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates;clusterissuers;issuers,verbs=get;list;watch;create;update
Comment on lines +103 to 105
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Deployments RBAC: create cannot be name-scoped; add global create or move to a namespaced Role.

As-is, operator won't be able to create its Deployments. Quick fix below; longer-term consider moving create to a namespaced Role bound to the operator SA.

-// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=list;watch
-// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;create;update,resourceNames=external-secrets;external-secrets-cert-controller;external-secrets-webhook;bitwarden-sdk-server
+// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;list;watch
+// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;update,resourceNames=external-secrets;external-secrets-cert-controller;external-secrets-webhook;bitwarden-sdk-server

Alternative (recommended hardening): move create/get/update for these resourceNames into a namespaced Role (CSV permissions) instead of ClusterRole.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=list;watch
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;create;update,resourceNames=external-secrets;external-secrets-cert-controller;external-secrets-webhook;bitwarden-sdk-server
// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates;clusterissuers;issuers,verbs=get;list;watch;create;update
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;list;watch
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;update,resourceNames=external-secrets;external-secrets-cert-controller;external-secrets-webhook;bitwarden-sdk-server
// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates;clusterissuers;issuers,verbs=get;list;watch;create;update
🤖 Prompt for AI Agents
In pkg/controller/external_secrets/controller.go around lines 103-105, the RBAC
kubebuilder comment scopes the create verb to specific resourceNames which is
invalid for create; fix by removing resourceNames from the create rule or by
moving the create permission into a namespaced Role bound to the operator
ServiceAccount. Concretely, update the RBAC annotation so ClusterRole grants
create (and other cluster-scoped verbs) on deployments without resourceNames, or
split responsibilities: keep get/update for the specific resourceNames in a Role
(namespaced) bound to the SA and grant create globally in the ClusterRole.

// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch;create

// +kubebuilder:rbac:groups="",resources=endpoints,verbs=get;list;watch;create
// +kubebuilder:rbac:groups="",resources=serviceaccounts/token,verbs=create
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;update;patch
// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets;clustersecretstores;clusterpushsecrets;externalsecrets;secretstores;pushsecrets,verbs=get;list;watch;create;update;patch;delete;deletecollection
// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/finalizers;clustersecretstores/finalizers;externalsecrets/finalizers;pushsecrets/finalizers;secretstores/finalizers;clusterpushsecrets/finalizers,verbs=get;update;patch
// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/status;clustersecretstores/status;externalsecrets/status;pushsecrets/status;secretstores/status;clusterpushsecrets/status,verbs=get;update;patch
Expand Down