Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve RBAC generation using JOSDK 4.9.5 features #965

Merged
merged 2 commits into from
Sep 25, 2024
Merged
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
26 changes: 13 additions & 13 deletions .github/workflows/build-for-quarkus-version.yml
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,13 @@ jobs:

- name: Check-out Fabric8 client PR if requested
uses: actions/checkout@v4
if: "${{ inputs.fkc-pr != '' }}"
if: "${{ inputs.fkc-pr != '' && inputs.fkc-version == '' }}"
with:
repository: fabric8io/kubernetes-client
path: fkc

- name: Build Fabric8 client PR if requested
if: "${{ inputs.fkc-pr != '' }}"
if: "${{ inputs.fkc-pr != '' && inputs.fkc-version == '' }}"
id: build-fkc-pr
run: |
cd fkc
Expand Down Expand Up @@ -232,11 +232,6 @@ jobs:
echo "Computed Maven profiles: ${maven_profiles}"
echo "maven_profiles=${maven_profiles}" >> $GITHUB_OUTPUT

- name: Change Quarkus version
run: |
echo "Using Quarkus ${{ steps.quarkus-version.outputs.quarkus_version }}"
./mvnw versions:set-property -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dproperty=quarkus.version -DnewVersion=${{ steps.quarkus-version.outputs.quarkus_version }}

- name: Change Fabric8 client version
id: fkc-version
if: "${{ inputs.fkc-pr != '' || inputs.fkc-version != '' }}"
Expand All @@ -251,22 +246,27 @@ jobs:
./mvnw versions:set-property -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dproperty=fabric8-client.version -DnewVersion=${fkc_version}
echo "fkc_version=${fkc_version}" >> $GITHUB_OUTPUT

- name: Change Quarkus version
run: |
echo "Using Quarkus ${{ steps.quarkus-version.outputs.quarkus_version }}"
./mvnw versions:set-property -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dproperty=quarkus.version -DnewVersion=${{ steps.quarkus-version.outputs.quarkus_version }} ${{inputs.mvnArgs}}

- name: Output versions being used
run: |
echo "QOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=project.version -q -DforceStdout)"
echo "JOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=java-operator-sdk.version -q -DforceStdout)"
echo "QOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=project.version -q -DforceStdout ${{inputs.mvnArgs}})"
echo "JOSDK version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=java-operator-sdk.version -q -DforceStdout ${{inputs.mvnArgs}})"
echo "JOSDK Fabric8 version: ${{ steps.build-josdk-pr.outputs.josdk_f8_version }}"
echo "JOSDK overridden Fabric8 version ${{ steps.fkc-version.outputs.fkc_version }}"
echo "Quarkus version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=quarkus.version -q -DforceStdout)"
echo "Quarkus version: $(./mvnw help:evaluate -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' -Dexpression=quarkus.version -q -DforceStdout ${{inputs.mvnArgs}})"
echo "Quarkus Fabric8 version: ${{ steps.build-quarkus-pr.outputs.quarkus_f8_version }}"
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)"
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)"

- name: Build with Maven (JVM)
run: ./mvnw -B formatter:validate install -P'${{steps.set-mvn-profiles.outputs.maven_profiles}}' ${{inputs.mvnArgs}} --file pom.xml

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

- name: Kubernetes KinD Cluster
uses: container-tools/kind-action@v2
Expand Down Expand Up @@ -332,7 +332,7 @@ jobs:
path: samples/joke/app.log

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

- name: Run Joke sample in native mode
if: ${{ contains(inputs.native-modules, 'samples') }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ private static Set<PolicyRule> getClusterRolePolicyRulesFromDependentResources(Q

// only process Kubernetes dependents
if (HasMetadata.class.isAssignableFrom(associatedResourceClass)) {
var resourceGroup = HasMetadata.getGroup(associatedResourceClass);
var resourcePlural = HasMetadata.getPlural(associatedResourceClass);
final var asHasMetadataClass = (Class<? extends HasMetadata>) associatedResourceClass;
var resourceGroup = HasMetadata.getGroup(asHasMetadataClass);
var resourcePlural = HasMetadata.getPlural(asHasMetadataClass);

final var verbs = new TreeSet<>(List.of(RBACVerbs.READ_VERBS));
if (Updater.class.isAssignableFrom(dependentResourceClass)) {
Expand All @@ -124,34 +125,24 @@ private static Set<PolicyRule> getClusterRolePolicyRulesFromDependentResources(Q
verbs.add(RBACVerbs.DELETE);
}
final var isCreator = Creator.class.isAssignableFrom(dependentResourceClass);
boolean shouldDoubleCheckPatch = false;
if (isCreator) {
verbs.add(RBACVerbs.CREATE);

// PATCH verb is also needed when using SSA to be able to add the finalizer when creating the resource
// Here, we optimistically add PATCH method if the resource configuration states that SSA should be
// used, despite this not being a correct/complete determination of whether the resource actually
// uses SSA. This can only be determined by instantiating the dependent, which is why, if we can
// instantiate it, we double-check the SSA status later on and remove the PATCH method if we can
// actually determine that it's not needed
final Object dependentResourceConfig = spec.getDependentResourceConfig();
if (dependentResourceConfig instanceof KubernetesDependentResourceConfig<?> kubernetesDependentResourceConfig) {
if (kubernetesDependentResourceConfig.useSSA().orElse(false)) {
verbs.add(RBACVerbs.PATCH);
shouldDoubleCheckPatch = true;
}
}
}

// Check if we're dealing with typeless Kubernetes resource or if we need to deal with SSA
boolean ignore = false;
KubernetesDependentResource<?, ?> kubeResource = null;
if (KubernetesDependentResource.class.isAssignableFrom(dependentResourceClass)) {
final var asKubeDRClass = (Class<? extends KubernetesDependentResource<?, ?>>) dependentResourceClass;

// PATCH is also required when creating resources to add finalizers when using SSA
if (isCreator && cri.getConfigurationService().shouldUseSSA(asKubeDRClass, asHasMetadataClass,
(KubernetesDependentResourceConfig<? extends HasMetadata>) spec.getDependentResourceConfig())) {
verbs.add(RBACVerbs.PATCH);
}

try {
//noinspection rawtypes
kubeResource = Utils.instantiate(
(Class<? extends KubernetesDependentResource>) dependentResourceClass,
KubernetesDependentResource.class, ADD_CLUSTER_ROLES_DECORATOR);
final var kubeResource = Utils.instantiate(asKubeDRClass, KubernetesDependentResource.class,
ADD_CLUSTER_ROLES_DECORATOR);

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

if (!ignore) {
// if we need to double check if we really should use SSA
if (shouldDoubleCheckPatch) {
// we can only check if we managed to instantiate the dependent, though
if (kubeResource != null) {
if (!cri.getConfigurationService().shouldUseSSA(kubeResource)) {
verbs.remove(RBACVerbs.PATCH);
}
} else {
// if we couldn't double check, warn the user
log.warn("Couldn't verify that dependent " + dependentResourceClass.getName()
+ " 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: "
+ resourceGroup + " / plural: " + resourcePlural + ") when not needed.");
}
}

final var dependentRule = new PolicyRuleBuilder()
.addToApiGroups(resourceGroup)
.addToResources(resourcePlural);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ quarkus.kubernetes-client.devservices.flavor=k3s
quarkus.http.test-port=0

quarkus.kubernetes-client.devservices.enabled=true
quarkus.operator-sdk.helm.enabled=true
quarkus.operator-sdk.helm.enabled=true

# todo: remove workaround for https://github.com/quarkusio/quarkus/issues/43474 when fixed
quarkus.kubernetes-client.request-retry-backoff-limit=10
5 changes: 4 additions & 1 deletion samples/joke/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ quarkus.container-image.builder=jib
quarkus.operator-sdk.bundle.channels=alpha
quarkus.operator-sdk.crd.generate-all=true
quarkus.kubernetes-client.devservices.override-kubeconfig=true
quarkus.http.test-port=0
quarkus.http.test-port=0

# todo: remove workaround for https://github.com/quarkusio/quarkus/issues/43474 when fixed
quarkus.kubernetes-client.request-retry-backoff-limit=10
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ quarkus.native.additional-build-args=--initialize-at-run-time=org.apache.commons
quarkus.datasource.username=root
quarkus.datasource.devservices.image-name=mariadb:10.7
quarkus.kubernetes-client.devservices.override-kubeconfig=true
quarkus.http.test-port=0
quarkus.http.test-port=0

# todo: remove workaround for https://github.com/quarkusio/quarkus/issues/43474 when fixed
quarkus.kubernetes-client.request-retry-backoff-limit=10
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ quarkus.operator-sdk.crd.apply=true
quarkus.container-image.builder=jib
quarkus.operator-sdk.bundle.channels=alpha
quarkus.kubernetes-client.devservices.override-kubeconfig=true
quarkus.http.test-port=0
quarkus.http.test-port=0

# todo: remove workaround for https://github.com/quarkusio/quarkus/issues/43474 when fixed
quarkus.kubernetes-client.request-retry-backoff-limit=10