-
Notifications
You must be signed in to change notification settings - Fork 231
feat: allow overriding test infrastructure kube client separately #2764
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
Changes from all commits
b7aa962
439d554
5196658
0357d0f
eafd20d
98cb8da
59e713e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ private ClusterDeployedOperatorExtension( | |
boolean waitForNamespaceDeletion, | ||
boolean oneNamespacePerClass, | ||
KubernetesClient kubernetesClient, | ||
KubernetesClient infrastructureKubernetesClient, | ||
Function<ExtensionContext, String> namespaceNameSupplier, | ||
Function<ExtensionContext, String> perClassNamespaceNameSupplier) { | ||
super( | ||
|
@@ -48,6 +49,7 @@ private ClusterDeployedOperatorExtension( | |
preserveNamespaceOnError, | ||
waitForNamespaceDeletion, | ||
kubernetesClient, | ||
infrastructureKubernetesClient, | ||
namespaceNameSupplier, | ||
perClassNamespaceNameSupplier); | ||
this.operatorDeployment = operatorDeployment; | ||
|
@@ -69,7 +71,7 @@ protected void before(ExtensionContext context) { | |
final var crdPath = "./target/classes/META-INF/fabric8/"; | ||
final var crdSuffix = "-v1.yml"; | ||
|
||
final var kubernetesClient = getKubernetesClient(); | ||
final var kubernetesClient = getInfrastructureKubernetesClient(); | ||
for (var crdFile : | ||
Objects.requireNonNull( | ||
new File(crdPath).listFiles((ignored, name) -> name.endsWith(crdSuffix)))) { | ||
|
@@ -107,13 +109,17 @@ protected void before(ExtensionContext context) { | |
|
||
@Override | ||
protected void deleteOperator() { | ||
getKubernetesClient().resourceList(operatorDeployment).inNamespace(namespace).delete(); | ||
getInfrastructureKubernetesClient() | ||
.resourceList(operatorDeployment) | ||
.inNamespace(namespace) | ||
.delete(); | ||
} | ||
|
||
public static class Builder extends AbstractBuilder<Builder> { | ||
private final List<HasMetadata> operatorDeployment; | ||
private Duration deploymentTimeout; | ||
private KubernetesClient kubernetesClient; | ||
private KubernetesClient infrastructureKubernetesClient; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @xstefank this seems to be legit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thx, my eyes skipped this comment |
||
|
||
protected Builder() { | ||
super(); | ||
|
@@ -150,7 +156,18 @@ public Builder withKubernetesClient(KubernetesClient kubernetesClient) { | |
return this; | ||
} | ||
|
||
public Builder withInfrastructureKubernetesClient(KubernetesClient kubernetesClient) { | ||
this.infrastructureKubernetesClient = kubernetesClient; | ||
return this; | ||
} | ||
|
||
public ClusterDeployedOperatorExtension build() { | ||
infrastructureKubernetesClient = | ||
infrastructureKubernetesClient != null | ||
? infrastructureKubernetesClient | ||
: new KubernetesClientBuilder().build(); | ||
kubernetesClient = | ||
kubernetesClient != null ? kubernetesClient : infrastructureKubernetesClient; | ||
return new ClusterDeployedOperatorExtension( | ||
operatorDeployment, | ||
deploymentTimeout, | ||
|
@@ -159,7 +176,8 @@ public ClusterDeployedOperatorExtension build() { | |
preserveNamespaceOnError, | ||
waitForNamespaceDeletion, | ||
oneNamespacePerClass, | ||
kubernetesClient != null ? kubernetesClient : new KubernetesClientBuilder().build(), | ||
kubernetesClient, | ||
infrastructureKubernetesClient, | ||
namespaceNameSupplier, | ||
perClassNamespaceNameSupplier); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
package io.javaoperatorsdk.operator.baseapi.infrastructureclient; | ||
|
||
import java.util.concurrent.TimeUnit; | ||
|
||
import org.junit.jupiter.api.AfterEach; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; | ||
import io.fabric8.kubernetes.api.model.rbac.ClusterRole; | ||
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; | ||
import io.fabric8.kubernetes.client.ConfigBuilder; | ||
import io.fabric8.kubernetes.client.KubernetesClientBuilder; | ||
import io.fabric8.kubernetes.client.KubernetesClientException; | ||
import io.javaoperatorsdk.operator.ReconcilerUtils; | ||
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
import static org.awaitility.Awaitility.await; | ||
|
||
class InfrastructureClientIT { | ||
|
||
private static final String RBAC_TEST_ROLE = "rbac-test-role.yaml"; | ||
private static final String RBAC_TEST_ROLE_BINDING = "rbac-test-role-binding.yaml"; | ||
private static final String RBAC_TEST_USER = "rbac-test-user"; | ||
|
||
@RegisterExtension | ||
LocallyRunOperatorExtension operator = | ||
LocallyRunOperatorExtension.builder() | ||
.withReconciler(new InfrastructureClientTestReconciler()) | ||
.withKubernetesClient( | ||
new KubernetesClientBuilder() | ||
.withConfig(new ConfigBuilder().withImpersonateUsername(RBAC_TEST_USER).build()) | ||
.build()) | ||
.withInfrastructureKubernetesClient( | ||
new KubernetesClientBuilder().build()) // no limitations | ||
.build(); | ||
|
||
/** | ||
* We need to apply the cluster role also before the CRD deployment so the rbac-test-user is | ||
* permitted to deploy it | ||
*/ | ||
public InfrastructureClientIT() { | ||
applyClusterRole(RBAC_TEST_ROLE); | ||
applyClusterRoleBinding(RBAC_TEST_ROLE_BINDING); | ||
} | ||
|
||
@BeforeEach | ||
void setup() { | ||
applyClusterRole(RBAC_TEST_ROLE); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are I guess redundant. The one from constructor might be an another hook I guess?! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the problem I mentioned to you on the call. If I removed this BeforeEach, the cluster roles wouldn't be applied for the second test. |
||
applyClusterRoleBinding(RBAC_TEST_ROLE_BINDING); | ||
} | ||
|
||
@AfterEach | ||
void cleanup() { | ||
removeClusterRoleBinding(RBAC_TEST_ROLE_BINDING); | ||
removeClusterRole(RBAC_TEST_ROLE); | ||
} | ||
|
||
@Test | ||
void canCreateInfrastructure() { | ||
var resource = new InfrastructureClientTestCustomResource(); | ||
resource.setMetadata( | ||
new ObjectMetaBuilder().withName("infrastructure-client-resource").build()); | ||
operator.create(resource); | ||
|
||
await() | ||
.atMost(5, TimeUnit.SECONDS) | ||
.untilAsserted( | ||
() -> { | ||
InfrastructureClientTestCustomResource r = | ||
operator.get( | ||
InfrastructureClientTestCustomResource.class, | ||
"infrastructure-client-resource"); | ||
assertThat(r).isNotNull(); | ||
}); | ||
|
||
assertThat( | ||
operator | ||
.getReconcilerOfType(InfrastructureClientTestReconciler.class) | ||
.getNumberOfExecutions()) | ||
.isEqualTo(1); | ||
} | ||
|
||
@Test | ||
void shouldNotAccessNotPermittedResources() { | ||
assertThatThrownBy( | ||
() -> | ||
operator | ||
.getKubernetesClient() | ||
.apiextensions() | ||
.v1() | ||
.customResourceDefinitions() | ||
.list()) | ||
.isInstanceOf(KubernetesClientException.class) | ||
.hasMessageContaining( | ||
"User \"%s\" cannot list resource \"customresourcedefinitions\"" | ||
.formatted(RBAC_TEST_USER)); | ||
|
||
// but we should be able to access all resources with the infrastructure client | ||
var deploymentList = | ||
operator | ||
.getInfrastructureKubernetesClient() | ||
.apiextensions() | ||
.v1() | ||
.customResourceDefinitions() | ||
.list(); | ||
assertThat(deploymentList).isNotNull(); | ||
} | ||
|
||
private void applyClusterRoleBinding(String filename) { | ||
var clusterRoleBinding = | ||
ReconcilerUtils.loadYaml(ClusterRoleBinding.class, this.getClass(), filename); | ||
operator.getInfrastructureKubernetesClient().resource(clusterRoleBinding).serverSideApply(); | ||
} | ||
|
||
private void applyClusterRole(String filename) { | ||
var clusterRole = ReconcilerUtils.loadYaml(ClusterRole.class, this.getClass(), filename); | ||
operator.getInfrastructureKubernetesClient().resource(clusterRole).serverSideApply(); | ||
} | ||
|
||
private void removeClusterRoleBinding(String filename) { | ||
var clusterRoleBinding = | ||
ReconcilerUtils.loadYaml(ClusterRoleBinding.class, this.getClass(), filename); | ||
operator.getInfrastructureKubernetesClient().resource(clusterRoleBinding).delete(); | ||
} | ||
|
||
private void removeClusterRole(String filename) { | ||
var clusterRole = ReconcilerUtils.loadYaml(ClusterRole.class, this.getClass(), filename); | ||
operator.getInfrastructureKubernetesClient().resource(clusterRole).delete(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package io.javaoperatorsdk.operator.baseapi.infrastructureclient; | ||
|
||
import io.fabric8.kubernetes.api.model.Namespaced; | ||
import io.fabric8.kubernetes.client.CustomResource; | ||
import io.fabric8.kubernetes.model.annotation.Group; | ||
import io.fabric8.kubernetes.model.annotation.ShortNames; | ||
import io.fabric8.kubernetes.model.annotation.Version; | ||
|
||
@Group("sample.javaoperatorsdk") | ||
@Version("v1") | ||
@ShortNames("ict") | ||
public class InfrastructureClientTestCustomResource extends CustomResource<Void, Void> | ||
implements Namespaced {} |
Uh oh!
There was an error while loading. Please reload this page.