Request single segment images when using local lookup ImageStreams

extensions/container-image/deployment/src/main/java/io/quarkus/container/image/deployment/ContainerImageConfig.java

@@ -15,22 +15,22 @@ public class ContainerImageConfig {
     /**
      * The group the container image will be part of
      */
-    @ConfigItem(defaultValue = "${user.name}")
+    @ConfigItem
     @ConvertWith(TrimmedStringConverter.class)
-    public Optional<String> group;
+    Optional<String> group; //used only by ContainerImageProcessor, use ContainerImageInfoBuildItem instead
 
     /**
      * The name of the container image. If not set defaults to the application name
      */
     @ConfigItem(defaultValue = "${quarkus.application.name:unset}")
     @ConvertWith(TrimmedStringConverter.class)
-    public String name;
+    String name; //used only by ContainerImageProcessor, use ContainerImageInfoBuildItem instead
 
     /**
      * The tag of the container image. If not set defaults to the application version
      */
     @ConfigItem(defaultValue = "${quarkus.application.version:latest}")
-    public Optional<String> tag;
+    Optional<String> tag; //used only by ContainerImageProcessor, use ContainerImageInfoBuildItem instead
 
     /**
      * Additional tags of the container image.
@@ -110,26 +110,4 @@ public boolean isPushExplicitlyEnabled() {
     public boolean isPushExplicitlyDisabled() {
         return push.isPresent() && !push.get();
     }
-
-    /**
-     * Since user.name which is default value can be uppercase and uppercase values are not allowed
-     * in the repository part of image references, we need to make the username lowercase.
-     * If spaces exist in the user name, we replace them with the dash character.
-     *
-     * We purposely don't change the value of an explicitly set group.
-     */
-    public Optional<String> getEffectiveGroup() {
-        if (group.isPresent()) {
-            String originalGroup = group.get();
-            if (originalGroup.equals(System.getProperty("user.name"))) {
-                if (originalGroup.isEmpty()) {
-                    return Optional.empty();
-                }
-                return Optional.of(originalGroup.toLowerCase().replace(' ', '-'));
-            } else if (originalGroup.isEmpty()) {
-                return Optional.empty();
-            }
-        }
-        return group;
-    }
 }

extensions/container-image/deployment/src/main/java/io/quarkus/container/image/deployment/ContainerImageProcessor.java

@@ -1,12 +1,15 @@
 package io.quarkus.container.image.deployment;
 
-import static io.quarkus.container.image.deployment.util.EnablementUtil.*;
+import static io.quarkus.container.image.deployment.util.EnablementUtil.buildOrPushContainerImageNeeded;
 import static io.quarkus.container.spi.ImageReference.DEFAULT_TAG;
 import static io.quarkus.deployment.builditem.ApplicationInfoBuildItem.UNSET_VALUE;
 
 import java.util.Collections;
 import java.util.Optional;
+import java.util.function.Predicate;
+import java.util.stream.StreamSupport;
 
+import org.eclipse.microprofile.config.ConfigProvider;
 import org.jboss.logging.Logger;
 
 import io.quarkus.container.spi.ContainerImageBuildRequestBuildItem;
@@ -15,17 +18,18 @@
 import io.quarkus.container.spi.ContainerImagePushRequestBuildItem;
 import io.quarkus.container.spi.FallbackContainerImageRegistryBuildItem;
 import io.quarkus.container.spi.ImageReference;
+import io.quarkus.container.spi.SingleSegmentContainerImageRequestBuildItem;
 import io.quarkus.deployment.Capabilities;
 import io.quarkus.deployment.annotations.BuildProducer;
 import io.quarkus.deployment.annotations.BuildStep;
 import io.quarkus.deployment.builditem.ApplicationInfoBuildItem;
 import io.quarkus.deployment.builditem.SuppressNonRuntimeConfigChangedWarningBuildItem;
 import io.quarkus.deployment.pkg.builditem.ArtifactResultBuildItem;
 import io.quarkus.deployment.pkg.steps.NativeSourcesBuild;
+import io.quarkus.runtime.util.StringUtil;
 
 public class ContainerImageProcessor {
 
-    private static final String UNKNOWN_USER = "?";
     private static final Logger log = Logger.getLogger(ContainerImageProcessor.class);
 
     @BuildStep(onlyIf = NativeSourcesBuild.class)
@@ -48,14 +52,16 @@ public void ignoreCredentialsChange(BuildProducer<SuppressNonRuntimeConfigChange
     @BuildStep
     public void publishImageInfo(ApplicationInfoBuildItem app,
             ContainerImageConfig containerImageConfig,
+            Optional<SingleSegmentContainerImageRequestBuildItem> singleSegmentImageRequest,
             Optional<FallbackContainerImageRegistryBuildItem> containerImageRegistry,
             Optional<ContainerImageCustomNameBuildItem> containerImageCustomName,
             Capabilities capabilities,
             BuildProducer<ContainerImageInfoBuildItem> containerImage) {
 
         ensureSingleContainerImageExtension(capabilities);
 
-        // additionalTags are used even containerImageConfig.image is set because that string cannot contain multiple tags
+        // additionalTags are used even containerImageConfig.image is set because that
+        // string cannot contain multiple tags
         if (containerImageConfig.additionalTags.isPresent()) {
             for (String additionalTag : containerImageConfig.additionalTags.get()) {
                 if (!ImageReference.isValidTag(additionalTag)) {
@@ -65,35 +71,37 @@ public void publishImageInfo(ApplicationInfoBuildItem app,
             }
         }
 
-        String effectiveGroup = containerImageConfig.getEffectiveGroup().orElse("");
-        //This can be the case when running inside kubernetes/minikube in dev-mode. Instead of failing, we should just catch and log.
-        if (UNKNOWN_USER.equals(effectiveGroup)) {
-            log.warn(
-                    "Can't get the current user name, which is used as default for container image group. Can't publish container image info.");
-            return;
-        }
+        Optional<String> effectiveGroup = getEffectiveGroup(containerImageConfig.group, singleSegmentImageRequest.isPresent());
 
         // if the user supplied the entire image string, use it
         if (containerImageConfig.image.isPresent()) {
             ImageReference imageReference = ImageReference.parse(containerImageConfig.image.get());
             String repository = imageReference.getRepository();
-            containerImage.produce(new ContainerImageInfoBuildItem(Optional.of(imageReference.getRegistry()), repository,
+            if (singleSegmentImageRequest.isPresent() && imageReference.getRepository().contains("/")
+                    && StringUtil.isNullOrEmpty(imageReference.getRegistry())) {
+                log.warn("A single segment image is preferred, but a local multi segment has been provided: "
+                        + containerImageConfig.image.get());
+            }
+            containerImage.produce(new ContainerImageInfoBuildItem(Optional.of(imageReference.getRegistry()),
+                    repository,
                     imageReference.getTag(), containerImageConfig.additionalTags.orElse(Collections.emptyList())));
             return;
         }
 
         String registry = containerImageConfig.registry
-                .orElseGet(() -> containerImageRegistry.map(FallbackContainerImageRegistryBuildItem::getRegistry).orElse(null));
+                .orElseGet(() -> containerImageRegistry.map(FallbackContainerImageRegistryBuildItem::getRegistry)
+                        .orElse(null));
         if ((registry != null) && !ImageReference.isValidRegistry(registry)) {
             throw new IllegalArgumentException("The supplied container-image registry '" + registry + "' is invalid");
         }
 
         String effectiveName = containerImageCustomName.map(ContainerImageCustomNameBuildItem::getName)
                 .orElse(containerImageConfig.name);
-        String repository = (containerImageConfig.getEffectiveGroup().map(s -> s + "/").orElse("")) + effectiveName;
+        String group = effectiveGroup.orElse("");
+        String repository = group.isBlank() ? effectiveName : group + "/" + effectiveName;
         if (!ImageReference.isValidRepository(repository)) {
             throw new IllegalArgumentException("The supplied combination of container-image group '"
-                    + effectiveGroup + "' and name '" + effectiveName + "' is invalid");
+                    + group + "' and name '" + effectiveName + "' is invalid");
         }
 
         String effectiveTag = containerImageConfig.tag.orElse(app.getVersion());
@@ -106,12 +114,43 @@ public void publishImageInfo(ApplicationInfoBuildItem app,
         }
 
         containerImage.produce(new ContainerImageInfoBuildItem(Optional.ofNullable(registry),
-                containerImageConfig.getEffectiveGroup(),
+                effectiveGroup,
                 effectiveName, effectiveTag,
                 containerImageConfig.additionalTags.orElse(Collections.emptyList())));
     }
 
     private void ensureSingleContainerImageExtension(Capabilities capabilities) {
         ContainerImageCapabilitiesUtil.getActiveContainerImageCapability(capabilities);
     }
+
+    /**
+     * Since user.name which is default value can be uppercase and uppercase values
+     * are not allowed
+     * in the repository part of image references, we need to make the username
+     * lowercase.
+     * If spaces exist in the user name, we replace them with the dash character.
+     *
+     * We purposely don't change the value of an explicitly set group.
+     */
+    static Optional<String> getEffectiveGroup(Optional<String> group, boolean isSingleSegmentRequested) {
+        return group.or(() -> isSingleSegmentRequested || isGroupSpecified()
+                ? Optional.empty()
+                : Optional.ofNullable(System.getProperty("user.name")).map(s -> s.replace(' ', '-')))
+                .map(String::toLowerCase)
+                .filter(Predicate.not(StringUtil::isNullOrEmpty));
+    }
+
+    static Optional<String> getEffectiveGroup() {
+        return getEffectiveGroup(Optional.empty(), false);
+    }
+
+    /**
+     * Users are allowed to specify an empty group, however this is mapped to Optional.emtpy().
+     * We need to know if the user has actually specified a group or not.
+     * The only way is to check the property names provided.
+     **/
+    static boolean isGroupSpecified() {
+        return StreamSupport.stream(ConfigProvider.getConfig().getPropertyNames().spliterator(), false)
+                .anyMatch(n -> n.equals("quarkus.container-image.group") || n.equals("QUARKUS_CONTAINER_IMAGE_GROUP"));
+    }
 }

extensions/container-image/deployment/src/test/java/io/quarkus/container/image/deployment/ContainerImageConfigEffectiveGroupTest.java

@@ -9,38 +9,37 @@
 class ContainerImageConfigEffectiveGroupTest {
 
     public static final String USER_NAME_SYSTEM_PROPERTY = "user.name";
-
-    private ContainerImageConfig sut = new ContainerImageConfig();
+    private final Optional<String> EMPTY = Optional.empty();
 
     @Test
     void testFromLowercaseUsername() {
         testWithNewUsername("test", () -> {
-            sut.group = Optional.of(System.getProperty(USER_NAME_SYSTEM_PROPERTY));
-            assertEquals(sut.getEffectiveGroup(), Optional.of("test"));
+            Optional<String> group = Optional.of(System.getProperty(USER_NAME_SYSTEM_PROPERTY));
+            assertEquals(ContainerImageProcessor.getEffectiveGroup(EMPTY, false), Optional.of("test"));
         });
     }
 
     @Test
     void testFromUppercaseUsername() {
         testWithNewUsername("TEST", () -> {
-            sut.group = Optional.of(System.getProperty(USER_NAME_SYSTEM_PROPERTY));
-            assertEquals(sut.getEffectiveGroup(), Optional.of("test"));
+            Optional<String> group = Optional.of(System.getProperty(USER_NAME_SYSTEM_PROPERTY));
+            assertEquals(ContainerImageProcessor.getEffectiveGroup(EMPTY, false), Optional.of("test"));
         });
     }
 
     @Test
     void testFromSpaceInUsername() {
         testWithNewUsername("user name", () -> {
-            sut.group = Optional.of(System.getProperty(USER_NAME_SYSTEM_PROPERTY));
-            assertEquals(sut.getEffectiveGroup(), Optional.of("user-name"));
+            Optional<String> group = Optional.of(System.getProperty(USER_NAME_SYSTEM_PROPERTY));
+            assertEquals(ContainerImageProcessor.getEffectiveGroup(EMPTY, false), Optional.of("user-name"));
         });
     }
 
     @Test
     void testFromOther() {
         testWithNewUsername("WhateveR", () -> {
-            sut.group = Optional.of("OtheR");
-            assertEquals(sut.getEffectiveGroup(), Optional.of("OtheR"));
+            Optional<String> group = Optional.of("OtheR");
+            assertEquals(ContainerImageProcessor.getEffectiveGroup(group, false), Optional.of("other"));
         });
     }
 

extensions/container-image/deployment/src/test/java/io/quarkus/container/image/deployment/ContainerImageInfoTest.java

@@ -122,7 +122,8 @@ private void whenPublishImageInfo() {
         Capabilities capabilities = new Capabilities(Collections.emptySet());
         BuildProducer<ContainerImageInfoBuildItem> containerImage = actualImageConfig -> actualContainerImageInfo = actualImageConfig;
         ContainerImageProcessor processor = new ContainerImageProcessor();
-        processor.publishImageInfo(app, containerImageConfig, Optional.empty(), Optional.empty(), capabilities, containerImage);
+        processor.publishImageInfo(app, containerImageConfig, Optional.empty(), Optional.empty(), Optional.empty(),
+                capabilities, containerImage);
     }
 
     private void thenImageIs(String expectedImage) {

extensions/container-image/spi/src/main/java/io/quarkus/container/spi/SingleSegmentContainerImageRequestBuildItem.java

@@ -0,0 +1,10 @@
+package io.quarkus.container.spi;
+
+import io.quarkus.builder.item.SimpleBuildItem;
+
+/**
+ * There are cases where a single segment image (an image without group) is preferred.
+ * This build item is used to express this preferrence.
+ **/
+public final class SingleSegmentContainerImageRequestBuildItem extends SimpleBuildItem {
+}

extensions/kubernetes/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/OpenshiftProcessor.java

@@ -43,6 +43,7 @@
 import io.quarkus.container.spi.ContainerImageInfoBuildItem;
 import io.quarkus.container.spi.ContainerImageLabelBuildItem;
 import io.quarkus.container.spi.FallbackContainerImageRegistryBuildItem;
+import io.quarkus.container.spi.SingleSegmentContainerImageRequestBuildItem;
 import io.quarkus.deployment.Capabilities;
 import io.quarkus.deployment.Capability;
 import io.quarkus.deployment.annotations.BuildProducer;
@@ -109,13 +110,15 @@ public void populateCustomImageName(OpenshiftConfig openshiftConfig,
     @BuildStep
     public void populateInternalRegistry(OpenshiftConfig openshiftConfig, ContainerImageConfig containerImageConfig,
             Capabilities capabilities,
-            BuildProducer<FallbackContainerImageRegistryBuildItem> containerImageRegistry) {
+            BuildProducer<FallbackContainerImageRegistryBuildItem> containerImageRegistry,
+            BuildProducer<SingleSegmentContainerImageRequestBuildItem> singleSegmentContainerImageRequest) {
 
         if (!containerImageConfig.registry.isPresent() && !containerImageConfig.image.isPresent()) {
             DeploymentResourceKind deploymentResourceKind = openshiftConfig.getDeploymentResourceKind(capabilities);
             if (deploymentResourceKind != DeploymentResourceKind.DeploymentConfig) {
                 if (openshiftConfig.isOpenshiftBuildEnabled(containerImageConfig, capabilities)) {
                     //Don't need fallback namespace, we use local lookup instead.
+                    singleSegmentContainerImageRequest.produce(new SingleSegmentContainerImageRequestBuildItem());
                 } else {
                     containerImageRegistry.produce(new FallbackContainerImageRegistryBuildItem(DOCKERIO_REGISTRY));
                 }

integration-tests/kubernetes/quarkus-standard-way/src/test/java/io/quarkus/it/kubernetes/OpenshiftWithDeploymentResourceAndLocalLookupTest.java

@@ -0,0 +1,99 @@
+
+package io.quarkus.it.kubernetes;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.apps.Deployment;
+import io.fabric8.openshift.api.model.ImageStream;
+import io.quarkus.builder.Version;
+import io.quarkus.maven.dependency.Dependency;
+import io.quarkus.test.ProdBuildResults;
+import io.quarkus.test.ProdModeTestResults;
+import io.quarkus.test.QuarkusProdModeTest;
+
+//
+// The purpose of this test is to assert that
+// When:
+//  - We run an in-cluster  container builds targeting Openshift (and `Deployment` is used).
+//  - No group / image is explicitly specified
+//
+// Then:
+//   - A BuildConfg is generated.
+//   - Two ImageStream are generated (one named after the app).
+//   - A Deployment resource was created
+//   - local lookup is enabled
+//   - single segement image is requested
+//
+public class OpenshiftWithDeploymentResourceAndLocalLookupTest {
+
+    private static final String NAME = "openshift-with-deployment-resource-and-local-lookup";
+
+    @RegisterExtension
+    static final QuarkusProdModeTest config = new QuarkusProdModeTest()
+            .withApplicationRoot((jar) -> jar.addClasses(GreetingResource.class))
+            .setApplicationName(NAME)
+            .setApplicationVersion("0.1-SNAPSHOT")
+            .overrideConfigKey("quarkus.openshift.deployment-kind", "Deployment")
+            .setLogFileName("k8s.log")
+            .setForcedDependencies(List.of(Dependency.of("io.quarkus", "quarkus-openshift", Version.getVersion())));
+
+    @ProdBuildResults
+    private ProdModeTestResults prodModeTestResults;
+
+    @Test
+    public void assertGeneratedResources() throws IOException {
+        final Path kubernetesDir = prodModeTestResults.getBuildDir().resolve("kubernetes");
+        assertThat(kubernetesDir)
+                .isDirectoryContaining(p -> p.getFileName().endsWith("openshift.json"))
+                .isDirectoryContaining(p -> p.getFileName().endsWith("openshift.yml"));
+        List<HasMetadata> kubernetesList = DeserializationUtil
+                .deserializeAsList(kubernetesDir.resolve("openshift.yml"));
+
+        assertThat(kubernetesList).filteredOn(h -> "BuildConfig".equals(h.getKind())).hasSize(1);
+        assertThat(kubernetesList).filteredOn(h -> "ImageStream".equals(h.getKind())).hasSize(2);
+        assertThat(kubernetesList).filteredOn(h -> "ImageStream".equals(h.getKind())
+                && h.getMetadata().getName().equals(NAME)).hasSize(1);
+
+        assertThat(kubernetesList).filteredOn(i -> i instanceof Deployment).singleElement().satisfies(i -> {
+            assertThat(i).isInstanceOfSatisfying(Deployment.class, d -> {
+                assertThat(d.getMetadata()).satisfies(m -> {
+                    assertThat(m.getName()).isEqualTo(NAME);
+                });
+
+                assertThat(d.getSpec()).satisfies(deploymentSpec -> {
+                    assertThat(deploymentSpec.getTemplate()).satisfies(t -> {
+                        assertThat(t.getMetadata()).satisfies(metadata -> assertThat(metadata.getLabels()).containsAnyOf(
+                                entry("app.kubernetes.io/name", NAME),
+                                entry("app.kubernetes.io/version", "0.1-SNAPSHOT")));
+
+                        assertThat(t.getSpec()).satisfies(podSpec -> {
+                            assertThat(podSpec.getContainers()).singleElement().satisfies(container -> {
+                                assertThat(container.getImage())
+                                        .isEqualTo("openshift-with-deployment-resource-and-local-lookup:0.1-SNAPSHOT");
+                            });
+                        });
+                    });
+                });
+            });
+        });
+
+        assertThat(kubernetesList).filteredOn(r -> r instanceof ImageStream && r.getMetadata().getName().equals(NAME))
+                .singleElement().satisfies(r -> {
+                    assertThat(r).isInstanceOfSatisfying(ImageStream.class, i -> {
+                        assertThat(i.getSpec()).satisfies(spec -> {
+                            assertThat(spec.getLookupPolicy().getLocal()).isEqualTo(true);
+                        });
+                    });
+                });
+
+    }
+}