-
Notifications
You must be signed in to change notification settings - Fork 0
Fix Helm chart values.yaml having github-container-registry as default imagePullSecret
#47
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
7419bab
baa3b99
5dda779
754edea
5b1ee90
2500549
53ce633
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 |
|---|---|---|
|
|
@@ -71,11 +71,29 @@ tasks.quarkusAppPartsBuild { | |
| doNotTrackState("Always execute Gradle task quarkusAppPartsBuild to generate the K8s deploy manifest kubernetes.yml, the CRDs, and to publish the Helm chart") | ||
| } | ||
|
|
||
| tasks.withType<Test> { | ||
| val mockitoAgent = configurations.testRuntimeClasspath.get().find { | ||
| it.name.contains("mockito-core") | ||
| } | ||
| if (mockitoAgent != null) { | ||
| jvmArgs("-javaagent:${mockitoAgent.absolutePath}") | ||
| val mockitoAgentProvider = configurations.named("testRuntimeClasspath").map { classpath -> | ||
| classpath.find { it.name.contains("mockito-core") } | ||
| } | ||
|
|
||
| tasks.withType<Test>().configureEach { | ||
| // Required for the HelmTest | ||
| dependsOn(tasks.quarkusAppPartsBuild) | ||
|
|
||
| jvmArgumentProviders.add(MockitoArgumentProvider(mockitoAgentProvider)) | ||
| } | ||
|
|
||
| class MockitoArgumentProvider( | ||
| @get:Optional | ||
| @get:InputFile | ||
| @get:PathSensitive(PathSensitivity.NONE) | ||
| val agentProvider: Provider<File> | ||
| ) : CommandLineArgumentProvider { | ||
| override fun asArguments(): Iterable<String> { | ||
| val agentFile = agentProvider.orNull | ||
| return if (agentFile != null) { | ||
| listOf("-javaagent:${agentFile.absolutePath}") | ||
| } else { | ||
| emptyList() | ||
| } | ||
| } | ||
|
Comment on lines
+85
to
98
Contributor
Author
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. We have to do this, else we get FAILURE: Build failed with an exception.
* What went wrong:
Configuration cache problems found in this build.
1 problem was found storing the configuration cache.
- Task `:operator:test` of type `org.gradle.api.tasks.testing.Test`: cannot serialize Gradle script object references as these are not supported with the configuration cache.
See https://docs.gradle.org/9.4.1/userguide/configuration_cache_requirements.html#config_cache:requirements:disallowed_typesGemini 3.1 ProThe error To make this fully compatible with the Configuration Cache, you need to use a distinct, explicitly defined class for the Here is the correct Configuration Cache compatible solution: // 1. Define a concrete class for the argument provider.
// This prevents capturing the Gradle script implicitly and tells Gradle exactly what the inputs are.
class MockitoArgumentProvider(
@get:Optional
@get:InputFile
@get:PathSensitive(PathSensitivity.NONE)
val agentProvider: Provider<File>
) : CommandLineArgumentProvider {
override fun asArguments(): Iterable<String> {
val agentFile = agentProvider.orNull
return if (agentFile != null) {
listOf("-javaagent:${agentFile.absolutePath}")
} else {
emptyList()
}
}
}
// 2. Resolve the provider lazily
val mockitoAgentProvider = configurations.named("testRuntimeClasspath").map { classpath ->
classpath.find { it.name.contains("mockito-core") }
}
tasks.withType<Test>().configureEach {
dependsOn(tasks.quarkusAppPartsBuild)
// 3. Add the provider class instance instead of a lambda
jvmArgumentProviders.add(MockitoArgumentProvider(mockitoAgentProvider))
}Why this fixes the issue:
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,3 +7,4 @@ spec: | |
| template: | ||
| spec: | ||
| affinity: {} | ||
| imagePullSecrets: [~] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,247 @@ | ||
| package it.aboutbits.postgresql.helm; | ||
|
|
||
| import io.fabric8.kubernetes.api.model.ConfigBuilder; | ||
| import io.fabric8.kubernetes.api.model.LocalObjectReference; | ||
| import io.fabric8.kubernetes.client.KubernetesClient; | ||
| import io.fabric8.kubernetes.client.utils.Serialization; | ||
| import io.quarkus.test.junit.QuarkusTest; | ||
| import io.smallrye.common.process.ProcessBuilder; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.eclipse.microprofile.config.inject.ConfigProperty; | ||
| import org.jspecify.annotations.NullMarked; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.awaitility.Awaitility.await; | ||
|
|
||
| @Slf4j | ||
| @QuarkusTest | ||
| @NullMarked | ||
| class HelmTest { | ||
| private static final String ENV_VAR_KUBECONFIG = "KUBECONFIG"; | ||
|
|
||
| private static final String CRD_GROUP = "postgresql.aboutbits.it"; | ||
| private static final List<String> CRD_NAMES = List.of( | ||
| "clusterconnection", | ||
| "database", | ||
| "schema", | ||
| "role", | ||
| "grant", | ||
| "defaultprivilege" | ||
| ); | ||
|
|
||
| private final String chartName; | ||
| private final String rootValuesAlias; | ||
| private final KubernetesClient kubernetesClient; | ||
|
|
||
| HelmTest( | ||
| KubernetesClient kubernetesClient, | ||
| @ConfigProperty(name = "quarkus.helm.name") String chartName, | ||
| @ConfigProperty(name = "quarkus.helm.values-root-alias", defaultValue = "app") String rootValuesAlias | ||
| ) { | ||
| this.kubernetesClient = kubernetesClient; | ||
| this.chartName = chartName; | ||
| this.rootValuesAlias = rootValuesAlias; | ||
| } | ||
|
|
||
| @SuppressWarnings("checkstyle:MethodLength") | ||
| @Test | ||
| @DisplayName("When the Helm chart is installed, the operator deployment should be created") | ||
| void helmInstall_createsDeployment() throws IOException { | ||
| // The chart is generated by the quarkus-helm extension in the build directory. | ||
| // For Gradle, it's build/helm/kubernetes/postgresql-operator | ||
| var chartPath = Paths.get("build", "helm", "kubernetes", chartName); | ||
|
|
||
| assertThat(chartPath) | ||
| .withFailMessage("Helm chart not found at %s. Ensure that the chart is generated before running this test.", chartPath) | ||
| .exists(); | ||
|
|
||
| // 1. Verify files exist and contain expected data | ||
| // ./Chart.yaml | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, Object> chartMetadata = Serialization.yamlMapper() | ||
| .readValue( | ||
| chartPath.resolve("Chart.yaml").toFile(), | ||
| Map.class | ||
| ); | ||
|
|
||
| assertThat(chartMetadata.get("name")).isEqualTo(chartName); | ||
|
|
||
| // ./values.yaml | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, Object> values = Serialization.yamlMapper() | ||
| .readValue( | ||
| chartPath.resolve("values.yaml").toFile(), | ||
| Map.class | ||
| ); | ||
|
|
||
| assertThat(values).containsKey(rootValuesAlias); | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| var appValues = (Map<String, Object>) values.get(rootValuesAlias); | ||
|
|
||
| Objects.requireNonNull(appValues, "appValues should not be null"); | ||
| assertThat(appValues.get("image")).isNotNull(); | ||
|
Comment on lines
+69
to
+94
Contributor
Author
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. |
||
|
|
||
| assertThat(chartPath.resolve("LICENSE")).exists(); | ||
| assertThat(chartPath.resolve("README.md")).exists(); | ||
| assertThat(chartPath.resolve("values.schema.json")).exists(); | ||
|
|
||
| // ./crds/ | ||
| for (var crdName : CRD_NAMES) { | ||
| assertThat(chartPath.resolve("crds/%ss.%s-v1.yml".formatted( | ||
| crdName, | ||
| CRD_GROUP | ||
| ))).exists(); | ||
| } | ||
|
|
||
| // ./templates/ | ||
| assertThat(chartPath.resolve("templates/clusterrole.yaml")).exists(); | ||
| assertThat(chartPath.resolve("templates/clusterrolebinding.yaml")).exists(); | ||
| assertThat(chartPath.resolve("templates/deployment.yaml")).exists(); | ||
| assertThat(chartPath.resolve("templates/rolebinding.yaml")).exists(); | ||
| assertThat(chartPath.resolve("templates/service.yaml")).exists(); | ||
| assertThat(chartPath.resolve("templates/serviceaccount.yaml")).exists(); | ||
| assertThat(chartPath.resolve("templates/validating-clusterrolebinding.yaml")).exists(); | ||
|
|
||
| for (var crdName : CRD_NAMES) { | ||
| assertThat(chartPath.resolve("templates/%sreconciler-crd-role-binding.yaml".formatted( | ||
| crdName | ||
| ))).exists(); | ||
| } | ||
|
|
||
| // 2. Prepare a temporary KubeConfig for the 'helm' CLI | ||
| // This ensures 'helm' uses the same Kubernetes cluster as the test environment (e.g., provided by DevServices). | ||
| var kubeConfigPath = createTempKubeConfig(); | ||
|
|
||
| try { | ||
| // 3. Install the Helm chart using 'helm install' | ||
| var releaseName = "helm-install-test-" + System.nanoTime(); | ||
|
|
||
| var holder = new Object() { | ||
| int exitCode; | ||
| }; | ||
| var installOutput = new StringBuilder(); | ||
|
|
||
| ProcessBuilder.newBuilder( | ||
| "helm", | ||
| "install", releaseName, chartPath.toAbsolutePath().toString(), "--set", rootValuesAlias + ".image=postgresql-operator:test" | ||
| ).environment(Map.of( | ||
| ENV_VAR_KUBECONFIG, | ||
| kubeConfigPath.toAbsolutePath().toString() | ||
| )) | ||
| .exitCodeChecker(ec -> { | ||
| holder.exitCode = ec; | ||
| return true; | ||
| }) | ||
| .error().redirect() | ||
| .output() | ||
| .consumeLinesWith(65536, line -> installOutput.append(line).append(System.lineSeparator())) | ||
| .run(); | ||
|
Comment on lines
+136
to
+150
Contributor
Author
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. I chose to use the See the discussion quarkusio/quarkus#48223 See the actions (PRs where they applied this concept) in Quarkus: |
||
|
|
||
| int installExitCode = holder.exitCode; | ||
| assertThat(installExitCode) | ||
| .withFailMessage("Helm install failed with output:\n" + installOutput) | ||
| .isZero(); | ||
|
|
||
| try { | ||
| // 4. Verify that the deployment is created in Kubernetes | ||
| await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> { | ||
| var deployment = kubernetesClient.apps().deployments().withName(chartName).get(); | ||
|
|
||
| assertThat(deployment).isNotNull(); | ||
|
|
||
| // Helm sets labels based on the release name | ||
| assertThat(deployment.getMetadata()) | ||
| .isNotNull() | ||
| .satisfies(metadata -> assertThat(metadata.getLabels()) | ||
| .containsAllEntriesOf(Map.of( | ||
| "app.kubernetes.io/name", releaseName, | ||
| "app.kubernetes.io/managed-by", "Helm" | ||
| )) | ||
| ); | ||
|
|
||
| assertThat(deployment.getSpec()) | ||
| .isNotNull() | ||
| .satisfies(spec -> assertThat(spec.getTemplate().getSpec().getImagePullSecrets()) | ||
| .isNotEmpty() | ||
| .element(0) | ||
| .isNotNull() | ||
| .isEqualTo(new LocalObjectReference(null)) | ||
| ); | ||
|
|
||
| var selector = deployment.getSpec().getSelector(); | ||
|
|
||
| var pods = kubernetesClient.pods() | ||
| .withLabelSelector(selector) | ||
| .list() | ||
| .getItems(); | ||
|
|
||
| assertThat(pods).isNotEmpty(); | ||
| }); | ||
| } finally { | ||
| // 5. Cleanup the created resources using 'helm uninstall' | ||
| ProcessBuilder.newBuilder( | ||
| "helm", | ||
| "uninstall", releaseName | ||
| ).environment(Map.of( | ||
| ENV_VAR_KUBECONFIG, | ||
| kubeConfigPath.toAbsolutePath().toString() | ||
| )) | ||
| .error().consumeLinesWith( | ||
| 8192, | ||
| log::error | ||
| ) | ||
| .run(); | ||
| } | ||
| } finally { | ||
| Files.deleteIfExists(kubeConfigPath); | ||
| } | ||
| } | ||
|
|
||
| private Path createTempKubeConfig() throws IOException { | ||
| var clientConfig = kubernetesClient.getConfiguration(); | ||
|
|
||
| var kubeConfig = new ConfigBuilder() | ||
| .addNewCluster() | ||
| .withName("dev-cluster") | ||
| .withNewCluster() | ||
| .withServer(clientConfig.getMasterUrl()) | ||
| .withCertificateAuthorityData(clientConfig.getCaCertData()) | ||
| .endCluster() | ||
| .endCluster() | ||
| .addNewUser() | ||
| .withName("dev-user") | ||
| .withNewUser() | ||
| .withClientCertificateData(clientConfig.getClientCertData()) | ||
| .withClientKeyData(clientConfig.getClientKeyData()) | ||
| .endUser() | ||
| .endUser() | ||
| .addNewContext() | ||
| .withName("dev-context") | ||
| .withNewContext() | ||
| .withCluster("dev-cluster") | ||
| .withUser("dev-user") | ||
| .withNamespace(clientConfig.getNamespace()) | ||
| .endContext() | ||
| .endContext() | ||
| .withCurrentContext("dev-context") | ||
| .build(); | ||
|
|
||
| var path = Files.createTempFile("kubeconfig-helm-test-", ".yaml"); | ||
|
|
||
| Files.writeString(path, Serialization.asYaml(kubeConfig)); | ||
|
|
||
| return path; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me
Why is it better to use this instead
of the following?
Gemini 3.1 Pro
The difference comes down to one of the most important concepts in modern Gradle: Eager vs. Lazy Evaluation (Configuration Avoidance).
Here is why
configurations.named("...").map { ... }is strictly better thanconfigurations.testRuntimeClasspath.get().find { ... }:1. The Configuration Phase vs. Execution Phase
Gradle builds happen in two main phases:
build.gradle.ktsfiles and builds a graph of all tasks and dependencies.2. What
.get()does (Eager Evaluation)When you use
configurations.testRuntimeClasspath.get(), you are telling Gradle:"Stop whatever you are doing and resolve all test dependencies right now, during the configuration phase."
The Problems:
gradlew tasksorgradlew clean)..get(), Gradle locks it. If another plugin or script later tries to add a dependency totestRuntimeClasspath, the build will crash with an error like: “Cannot change configuration 'testRuntimeClasspath' after it has been resolved.”3. What
.named().map()does (Lazy Evaluation)When you use
configurations.named("testRuntimeClasspath").map { ... }, you are creating aProvider.You are telling Gradle:
"I will need this file eventually, but don't figure it out right now. Here are the instructions on how to find it when the time comes."
The Benefits:
.find { ... }search only happen during the Execution Phase, and only if you actually run a task that needs it (likegradlew test). If you rungradlew build, and the test task is up-to-date or skipped, the resolution never happens at all.testRuntimeClasspathconfiguration before the execution phase begins.In summary,
.get()forces Gradle to do heavy work immediately (which can break things and slow down your build), while.named().map()postpones the work until the exact moment it is actually required.