Skip to content

Commit

Permalink
fix (jkube-kit/enricher) : ServiceAccountEnricher handles `serviceAcc…
Browse files Browse the repository at this point in the history 
…ount` configuration option (eclipse-jkube#2187)

ServiceAccountEnricher used to only handle `serviceAccounts`
resource configuration option. `serviceAccount` used to be used by
PodTemplateHandler while generating opinionated controllers.

Add a case for handling `serviceAccount` in ServiceAccountEnricher

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
@rohanKanojia
rohanKanojia committed Jun 1, 2023
1 parent d1efdb0 commit c301fe3
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -28,6 +28,7 @@ Usage:
* Fix #2166: Potential command line injection in SpringBootWatcher
* Fix #2170: `internal-microservice` profile prevents Service exposure
* Fix #2174: Profile merge constructor accounts for parentProfile field
* Fix #2187: `serviceAccount` configuration option has stopped working

### 1.12.0 (2023-04-03)
* Fix #1179: Move storageClass related functionality out of VolumePermissionEnricher to PersistentVolumeClaimStorageClassEnricher
Expand Down
4 changes: 2 additions & 2 deletions ...src/test/java/org/eclipse/jkube/gradle/plugin/tests/ServiceAccountNameViaGroovyDSLIT.java
View file
Expand Up @@ -36,8 +36,8 @@ class ServiceAccountNameViaGroovyDSLIT {

static Stream<Arguments> testInput() {
return Stream.of(
// arguments("no-fragment", new String[] {}),
// arguments("unrelated-fragment", new String[] {"-Pjkube.resourceDir=./regularFragmentDir"}),
arguments("no-fragment", new String[] {}),
arguments("unrelated-fragment", new String[] {"-Pjkube.resourceDir=./regularFragmentDir"}),
arguments("fragment-overriding-serviceaccount", new String[] {"-Pjkube.resourceDir=./overridingServiceAccountNameFragmentDir"})
);
}
Expand Down
23 changes: 16 additions & 7 deletions ...cher/generic/src/main/java/org/eclipse/jkube/enricher/generic/ServiceAccountEnricher.java
View file
Expand Up @@ -16,12 +16,15 @@
import io.fabric8.kubernetes.api.builder.TypedVisitor;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.ServiceAccount;
import io.fabric8.kubernetes.api.model.ServiceAccountBuilder;
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
import lombok.AllArgsConstructor;
import lombok.Getter;
import org.apache.commons.lang3.StringUtils;
import org.eclipse.jkube.kit.common.Configs;
import org.eclipse.jkube.kit.common.util.JKubeProjectUtil;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
import org.eclipse.jkube.kit.config.resource.ResourceConfig;
import org.eclipse.jkube.kit.config.resource.ServiceAccountConfig;
Expand Down Expand Up @@ -61,6 +64,9 @@ public void create(PlatformMode mode, KubernetesListBuilder builder) {
.filter(sa -> sa.getDeploymentRef() != null)
.collect(Collectors.toMap(ServiceAccountConfig::getDeploymentRef, ServiceAccountConfig::getName)));
}
if (resourceConfig != null && StringUtils.isNotBlank(resourceConfig.getServiceAccount())) {
deploymentToSaPair.put(JKubeProjectUtil.createDefaultResourceName(getContext().getGav().getSanitizedArtifactId()), resourceConfig.getServiceAccount());
}
builder.addAllToServiceAccountItems(createServiceAccountFromResourceConfig(resourceConfig));
builder.addAllToServiceAccountItems(createServiceAccountsReferencedInDeployment(builder, deploymentToSaPair));
}
Expand Down Expand Up @@ -88,13 +94,16 @@ public void visit(DeploymentBuilder deploymentBuilder) {
serviceAccounts.add(createServiceAccount(serviceAccountName));
}
if(deploymentToSaPair.containsKey(deploymentBuilder.buildMetadata().getName())) {
deploymentBuilder.editSpec()
.editTemplate()
.editSpec()
.withServiceAccountName(deploymentToSaPair.get(deploymentBuilder.buildMetadata().getName()))
.endSpec()
.endTemplate()
.endSpec();
PodSpec podSpec = deploymentBuilder.buildSpec().getTemplate().getSpec();
if (StringUtils.isBlank(podSpec.getServiceAccount()) && StringUtils.isBlank(podSpec.getServiceAccountName())) {
deploymentBuilder.editSpec()
.editTemplate()
.editSpec()
.withServiceAccountName(deploymentToSaPair.get(deploymentBuilder.buildMetadata().getName()))
.endSpec()
.endTemplate()
.endSpec();
}
}
}
});
Expand Down
57 changes: 54 additions & 3 deletions .../generic/src/test/java/org/eclipse/jkube/enricher/generic/ServiceAccountEnricherTest.java
View file
Expand Up @@ -13,11 +13,14 @@
*/
package org.eclipse.jkube.enricher.generic;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.ServiceAccount;
import io.fabric8.kubernetes.api.model.ServiceAccountBuilder;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
import io.fabric8.kubernetes.api.model.apps.DeploymentSpec;
import io.fabric8.kubernetes.api.model.PodTemplateSpec;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.eclipse.jkube.kit.common.JavaProject;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
Expand All @@ -26,10 +29,16 @@
import org.eclipse.jkube.kit.enricher.api.JKubeEnricherContext;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.Optional;
import java.util.Properties;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.params.provider.Arguments.arguments;

class ServiceAccountEnricherTest {
private JKubeEnricherContext context;
Expand Down Expand Up @@ -128,6 +137,34 @@ void create_withSkipCreateEnabledAndFragment_shouldNotCreateServiceAccount() {
.hasOnlyElementsOfType(Deployment.class);
}

@ParameterizedTest(name = "resources serviceAccount={0}, deployment, then generated deployment {2} is {3}")
@MethodSource("resourceConfigTestData")
void create_whenServiceAccountResourceConfigProvided_thenServiceAccountSetInGeneratedController(String resourceConfigServiceAccount, DeploymentBuilder deploymentFragment, String serviceAccountField, String serviceAccountName) {
// Given
context.getProperties().put("jkube.enricher.jkube-serviceaccount.skipCreate", "true");
context = context.toBuilder()
.resources(ResourceConfig.builder().serviceAccount(resourceConfigServiceAccount).build())
.project(JavaProject.builder().groupId("org.example").artifactId("cheese").version("0.0.1").build())
.build();
final KubernetesListBuilder builder = new KubernetesListBuilder();
builder.addToItems(deploymentFragment);

// When
new ServiceAccountEnricher(context).create(PlatformMode.kubernetes, builder);

// Then
assertControllerHasServiceAccountFieldSet(builder, serviceAccountField, serviceAccountName);
}

private static Stream<Arguments> resourceConfigTestData() {
return Stream.of(
arguments("", createNewDeploymentFragment(), "serviceAccountName", null),
arguments("sa-from-config", createNewDeploymentFragment(), "serviceAccountName", "sa-from-config"),
arguments("sa-from-config", createNewDeploymentFragmentWithServiceAccountConfigured("sa-from-fragment"), "serviceAccount", "sa-from-fragment"),
arguments("sa-from-config", createNewDeploymentFragmentWithServiceAccountNameConfigured("sa-from-fragment"), "serviceAccountName","sa-from-fragment")
);
}

private void enrichAndAssert(KubernetesListBuilder builder, String expectedServiceAccountName) {
final ServiceAccountEnricher saEnricher = new ServiceAccountEnricher(context);
saEnricher.create(PlatformMode.kubernetes, builder);
Expand All @@ -145,7 +182,7 @@ private void givenServiceAccountConfiguredInResourceConfiguration() {
.build();
}

private DeploymentBuilder createNewDeploymentFragment() {
private static DeploymentBuilder createNewDeploymentFragment() {
return new DeploymentBuilder()
.withNewMetadata().withName("cheese").endMetadata()
.withNewSpec()
Expand All @@ -157,7 +194,7 @@ private DeploymentBuilder createNewDeploymentFragment() {
.endSpec();
}

private DeploymentBuilder createNewDeploymentFragmentWithServiceAccountConfigured(String serviceAccount) {
private static DeploymentBuilder createNewDeploymentFragmentWithServiceAccountConfigured(String serviceAccount) {
return createNewDeploymentFragment().editSpec()
.editTemplate()
.editSpec()
Expand All @@ -167,7 +204,7 @@ private DeploymentBuilder createNewDeploymentFragmentWithServiceAccountConfigure
.endSpec();
}

private DeploymentBuilder createNewDeploymentFragmentWithServiceAccountNameConfigured(String serviceAccountName) {
private static DeploymentBuilder createNewDeploymentFragmentWithServiceAccountNameConfigured(String serviceAccountName) {
return createNewDeploymentFragment().editSpec()
.editTemplate()
.editSpec()
Expand All @@ -176,4 +213,18 @@ private DeploymentBuilder createNewDeploymentFragmentWithServiceAccountNameConfi
.endTemplate()
.endSpec();
}

private void assertControllerHasServiceAccountFieldSet(KubernetesListBuilder builder, String serviceAccountField, String serviceAccountName) {
Optional<HasMetadata> hasMetadataOptional = builder.buildItems()
.stream()
.filter(h -> h.getKind().equalsIgnoreCase("Deployment"))
.findAny();
assertThat(hasMetadataOptional)
.isPresent()
.get(InstanceOfAssertFactories.type(Deployment.class))
.extracting(Deployment::getSpec)
.extracting(DeploymentSpec::getTemplate)
.extracting(PodTemplateSpec::getSpec)
.hasFieldOrPropertyWithValue(serviceAccountField, serviceAccountName);
}
}

0 comments on commit c301fe3

Please sign in to comment.