Skip to content

Commit 87187c7

Browse files
metacosmmetacosm
committed
refactor: simplify SSA check in RBAC generation
Signed-off-by: Chris Laprun <claprun@redhat.com>
1 parent 149ae31 commit 87187c7

File tree

2 files changed

+26
-50
lines changed

2 files changed

+26
-50
lines changed

‎.github/workflows/build-for-quarkus-version.yml

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,13 @@ jobs:
183183
184184
- name: Check-out Fabric8 client PR if requested
185185
uses: actions/checkout@v4
186-
if: "${{ inputs.fkc-pr != '' }}"
186+
if: "${{ inputs.fkc-pr != '' && inputs.fkc-version == '' }}"
187187
with:
188188
repository: fabric8io/kubernetes-client
189189
path: fkc
190190

191191
- name: Build Fabric8 client PR if requested
192-
if: "${{ inputs.fkc-pr != '' }}"
192+
if: "${{ inputs.fkc-pr != '' && inputs.fkc-version == '' }}"
193193
id: build-fkc-pr
194194
run: |
195195
cd fkc
@@ -232,11 +232,6 @@ jobs:
232232
echo "Computed Maven profiles: ${maven_profiles}"
233233
echo "maven_profiles=${maven_profiles}" >> $GITHUB_OUTPUT
234234
235-
- name: Change Quarkus version
236-
run: |
237-
echo "Using Quarkus ${{ steps.quarkus-version.outputs.quarkus_version }}"
238-
./mvnw versions:set-property -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dproperty=quarkus.version -DnewVersion=${{ steps.quarkus-version.outputs.quarkus_version }}
239-
240235
- name: Change Fabric8 client version
241236
id: fkc-version
242237
if: "${{ inputs.fkc-pr != '' || inputs.fkc-version != '' }}"
@@ -251,22 +246,27 @@ jobs:
251246
./mvnw versions:set-property -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dproperty=fabric8-client.version -DnewVersion=${fkc_version}
252247
echo "fkc_version=${fkc_version}" >> $GITHUB_OUTPUT
253248
249+
- name: Change Quarkus version
250+
run: |
251+
echo "Using Quarkus ${{ steps.quarkus-version.outputs.quarkus_version }}"
252+
./mvnw versions:set-property -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dproperty=quarkus.version -DnewVersion=${{ steps.quarkus-version.outputs.quarkus_version }} ${{inputs.mvnArgs}}
253+
254254
- name: Output versions being used
255255
run: |
256-
echo "QOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=project.version -q -DforceStdout)"
257-
echo "JOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=java-operator-sdk.version -q -DforceStdout)"
256+
echo "QOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=project.version -q -DforceStdout ${{inputs.mvnArgs}})"
257+
echo "JOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=java-operator-sdk.version -q -DforceStdout ${{inputs.mvnArgs}})"
258258
echo "JOSDK Fabric8 version: ${{ steps.build-josdk-pr.outputs.josdk_f8_version }}"
259259
echo "JOSDK overridden Fabric8 version ${{ steps.fkc-version.outputs.fkc_version }}"
260-
echo "Quarkus version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=quarkus.version -q -DforceStdout)"
260+
echo "Quarkus version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=quarkus.version -q -DforceStdout ${{inputs.mvnArgs}})"
261261
echo "Quarkus Fabric8 version: ${{ steps.build-quarkus-pr.outputs.quarkus_f8_version }}"
262-
echo "Effective Fabric8 version: $(./mvnw dependency:tree -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dincludes=io.fabric8:kubernetes-client-api -pl core/deployment | grep io.fabric8:kubernetes-client-api -m1 | cut -d ':' -f 4)"
262+
echo "Effective Fabric8 version: $(./mvnw dependency:tree -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' ${{inputs.mvnArgs}} -Dincludes=io.fabric8:kubernetes-client-api -pl core/deployment | grep io.fabric8:kubernetes-client-api -m1 | cut -d ':' -f 4)"
263263
264264
- name: Build with Maven (JVM)
265265
run: ./mvnw -B formatter:validate install -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' ${{inputs.mvnArgs}} --file pom.xml
266266

267267
- name: Dependency tree on failure
268268
if: failure()
269-
run: ./mvnw -B dependency:tree -Dverbose
269+
run: ./mvnw -B dependency:tree -Dverbose ${{inputs.mvnArgs}}
270270

271271
- name: Kubernetes KinD Cluster
272272
uses: container-tools/kind-action@v2
@@ -332,7 +332,7 @@ jobs:
332332
path: samples/joke/app.log
333333

334334
- name: Build with Maven (Native)
335-
run: ./mvnw -B install -Dnative --file pom.xml -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -pl '${{inputs.native-modules}}' -amd
335+
run: ./mvnw -B install -Dnative --file pom.xml -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -pl '${{inputs.native-modules}}' -amd ${{inputs.mvnArgs}}
336336

337337
- name: Run Joke sample in native mode
338338
if: ${{ contains(inputs.native-modules, 'samples') }}

‎core/deployment/src/main/java/io/quarkiverse/operatorsdk/deployment/AddClusterRolesDecorator.java

Lines changed: 13 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,9 @@ private static Set<PolicyRule> getClusterRolePolicyRulesFromDependentResources(Q
113113

114114
// only process Kubernetes dependents
115115
if (HasMetadata.class.isAssignableFrom(associatedResourceClass)) {
116-
var resourceGroup = HasMetadata.getGroup(associatedResourceClass);
117-
var resourcePlural = HasMetadata.getPlural(associatedResourceClass);
116+
final var asHasMetadataClass = (Class<? extends HasMetadata>) associatedResourceClass;
117+
var resourceGroup = HasMetadata.getGroup(asHasMetadataClass);
118+
var resourcePlural = HasMetadata.getPlural(asHasMetadataClass);
118119

119120
final var verbs = new TreeSet<>(List.of(RBACVerbs.READ_VERBS));
120121
if (Updater.class.isAssignableFrom(dependentResourceClass)) {
@@ -124,34 +125,24 @@ private static Set<PolicyRule> getClusterRolePolicyRulesFromDependentResources(Q
124125
verbs.add(RBACVerbs.DELETE);
125126
}
126127
final var isCreator = Creator.class.isAssignableFrom(dependentResourceClass);
127-
boolean shouldDoubleCheckPatch = false;
128128
if (isCreator) {
129129
verbs.add(RBACVerbs.CREATE);
130-
131-
// PATCH verb is also needed when using SSA to be able to add the finalizer when creating the resource
132-
// Here, we optimistically add PATCH method if the resource configuration states that SSA should be
133-
// used, despite this not being a correct/complete determination of whether the resource actually
134-
// uses SSA. This can only be determined by instantiating the dependent, which is why, if we can
135-
// instantiate it, we double-check the SSA status later on and remove the PATCH method if we can
136-
// actually determine that it's not needed
137-
final Object dependentResourceConfig = spec.getDependentResourceConfig();
138-
if (dependentResourceConfig instanceof KubernetesDependentResourceConfig<?> kubernetesDependentResourceConfig) {
139-
if (kubernetesDependentResourceConfig.useSSA().orElse(false)) {
140-
verbs.add(RBACVerbs.PATCH);
141-
shouldDoubleCheckPatch = true;
142-
}
143-
}
144130
}
145131

146132
// Check if we're dealing with typeless Kubernetes resource or if we need to deal with SSA
147133
boolean ignore = false;
148-
KubernetesDependentResource<?, ?> kubeResource = null;
149134
if (KubernetesDependentResource.class.isAssignableFrom(dependentResourceClass)) {
135+
final var asKubeDRClass = (Class<? extends KubernetesDependentResource<?, ?>>) dependentResourceClass;
136+
137+
// PATCH is also required when creating resources to add finalizers when using SSA
138+
if (isCreator && cri.getConfigurationService().shouldUseSSA(asKubeDRClass, asHasMetadataClass,
139+
(KubernetesDependentResourceConfig<? extends HasMetadata>) spec.getDependentResourceConfig())) {
140+
verbs.add(RBACVerbs.PATCH);
141+
}
142+
150143
try {
151-
//noinspection rawtypes
152-
kubeResource = Utils.instantiate(
153-
(Class<? extends KubernetesDependentResource>) dependentResourceClass,
154-
KubernetesDependentResource.class, ADD_CLUSTER_ROLES_DECORATOR);
144+
final var kubeResource = Utils.instantiate(asKubeDRClass, KubernetesDependentResource.class,
145+
ADD_CLUSTER_ROLES_DECORATOR);
155146

156147
if (kubeResource instanceof GenericKubernetesDependentResource<? extends HasMetadata> genericKubeRes) {
157148
final var gvk = genericKubeRes.getGroupVersionKind();
@@ -166,21 +157,6 @@ private static Set<PolicyRule> getClusterRolePolicyRulesFromDependentResources(Q
166157
}
167158

168159
if (!ignore) {
169-
// if we need to double check if we really should use SSA
170-
if (shouldDoubleCheckPatch) {
171-
// we can only check if we managed to instantiate the dependent, though
172-
if (kubeResource != null) {
173-
if (!cri.getConfigurationService().shouldUseSSA(kubeResource)) {
174-
verbs.remove(RBACVerbs.PATCH);
175-
}
176-
} else {
177-
// if we couldn't double check, warn the user
178-
log.warn("Couldn't verify that dependent " + dependentResourceClass.getName()
179-
+ " really needs PATCH permission for SSA because it couldn't be instantiated. This means that a PATCH verb might have been added to the rule (group: "
180-
+ resourceGroup + " / plural: " + resourcePlural + ") when not needed.");
181-
}
182-
}
183-
184160
final var dependentRule = new PolicyRuleBuilder()
185161
.addToApiGroups(resourceGroup)
186162
.addToResources(resourcePlural);

0 commit comments

Comments
 (0)