Fix getEffectiveRegistry to return registry only (#569)

Author: jonesbusy

src/main/java/land/oras/ContainerRef.java

@@ -401,9 +401,14 @@ public static ContainerRef parse(String name) {
      */
     public String getEffectiveRegistry(Registry target) {
         if (isUnqualified()) {
-            String key = getAliasKey();
+            String key = target.getRegistriesConf().getAliasKey(this);
             if (target.getRegistry() == null && target.getRegistriesConf().hasAlias(key)) {
-                return target.getRegistriesConf().getAliases().get(key);
+                // Extract everything before the first slash (if any) as the registry for alias lookup, otherwise use
+                // the repository as the key
+                String value = target.getRegistriesConf().getAliases().get(key);
+                String domain = value.split("/")[0];
+                LOG.debug("Effective registry for alias {} is {}", key, domain);
+                return domain;
             }
             return target.getRegistry() != null
                     ? target.getRegistry()
@@ -489,7 +494,7 @@ public String getTarget(OCI<ContainerRef> target) {
      */
     public ContainerRef forRegistry(Registry registry) {
         if (isUnqualified() && registry.getRegistry() == null) {
-            String key = getAliasKey();
+            String key = registry.getRegistriesConf().getAliasKey(this);
             if (registry.getRegistry() == null && registry.getRegistriesConf().hasAlias(key)) {
                 String newLocation = registry.getRegistriesConf().getAliases().get(key);
                 String newRefString = "%s:%s".formatted(newLocation, tag);
@@ -514,15 +519,6 @@ public ContainerRef forRegistry(Registry registry) {
                 digest);
     }
 
-    /**
-     * Return the key of the alias
-     */
-    private String getAliasKey() {
-        return getRegistry().equals(Const.DEFAULT_REGISTRY) && "library".equals(getNamespace())
-                ? getRepository()
-                : getFullRepository();
-    }
-
     private String determineFirstUnqualifiedSearchRegistry(Registry registry) {
         // No settings, keep old behavior of defaulting to docker.io for unqualified reference
         if (registry.getRegistriesConf().getUnqualifiedRegistries().isEmpty()) {

src/main/java/land/oras/auth/RegistriesConf.java

@@ -35,6 +35,7 @@
 import java.util.Optional;
 import land.oras.ContainerRef;
 import land.oras.exception.OrasException;
+import land.oras.utils.Const;
 import land.oras.utils.TomlUtils;
 import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
@@ -192,6 +193,17 @@ public List<String> getUnqualifiedRegistries() {
         return Collections.unmodifiableList(config.unqualifiedRegistries);
     }
 
+    /**
+     * Return the key of the alias
+     * @param ref the container reference to get the alias key for.
+     * @return the alias key for the given container reference, which is either the repository name
+     */
+    public String getAliasKey(ContainerRef ref) {
+        return ref.getRegistry().equals(Const.DEFAULT_REGISTRY) && "library".equals(ref.getNamespace())
+                ? ref.getRepository()
+                : ref.getFullRepository();
+    }
+
     /**
      * Enforce the short name mode by checking the configuration. If the short name mode is set to ENFORCING or PERMISSIVE and there are multiple unqualified registries configured, this method throws an OrasException indicating that the configuration is invalid. If the configuration is valid, this method does nothing.
      * @throws OrasException if the short name mode is set to ENFORCING or PERMISSIVE and there are multiple unqualified registries configured, indicating that the configuration is invalid.
@@ -250,6 +262,7 @@ public boolean isInsecure(ContainerRef location) {
      * @return the rewritten container reference.
      */
     public ContainerRef rewrite(ContainerRef ref) {
+
         Optional<RegistryConfig> matchingConfig = selectMatchingTable(ref);
         if (matchingConfig.isEmpty()) {
             return ref;

src/test/java/land/oras/ContainerRefTest.java

@@ -85,18 +85,18 @@ void shouldDetermineFromAlias(@TempDir Path homeDir) throws Exception {
         String config =
                 """
             [aliases]
-            "my-library/my-namespace"="localhost/test"
-            "my-library"="localhost/test2"
+            "my-library/my-namespace"="localhost:5000/test"
+            "my-library"="localhost:5000/test2"
             """;
 
         TestUtils.createRegistriesConfFile(homeDir, config);
 
         TestUtils.withHome(homeDir, () -> {
             Registry registry = Registry.builder().defaults().build();
             ContainerRef unqualifiedRef = ContainerRef.parse("my-library/my-namespace");
-            assertEquals("localhost/test", unqualifiedRef.getEffectiveRegistry(registry));
+            assertEquals("localhost:5000", unqualifiedRef.getEffectiveRegistry(registry));
             ContainerRef unqualifiedRef2 = ContainerRef.parse("my-library");
-            assertEquals("localhost/test2", unqualifiedRef2.getEffectiveRegistry(registry));
+            assertEquals("localhost:5000", unqualifiedRef2.getEffectiveRegistry(registry));
         });
     }
 

src/test/java/land/oras/RegistryTest.java

@@ -40,6 +40,7 @@
 import land.oras.utils.ZotUnsecureContainer;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.api.parallel.Execution;
@@ -862,6 +863,64 @@ void testShouldCopySingleArtifactFromRegistryIntoRegistry() throws IOException {
         }
     }
 
+    @Test
+    @Execution(ExecutionMode.SAME_THREAD)
+    @Disabled("#")
+    void testShouldCopyFromAliasToAlias(@TempDir Path homeDir) throws Exception {
+
+        try (RegistryContainer otherRegistryContainer = new RegistryContainer()) {
+
+            otherRegistryContainer.start();
+
+            // language=toml
+            String config =
+                    """
+                [aliases]
+                "the-source" = "%s/test/artifact-source"
+                "the-target" = "%s/test/artifact-target"
+
+                [[registry]]
+                location = "%s"
+                insecure = true
+
+                [[registry]]
+                location = "%s"
+                insecure = true
+                """
+                            .formatted(
+                                    this.registry.getRegistry(),
+                                    otherRegistryContainer.getRegistry(),
+                                    this.registry.getRegistry(),
+                                    otherRegistryContainer.getRegistry());
+            TestUtils.createRegistriesConfFile(homeDir, config);
+
+            // Copy to same registry
+            TestUtils.withHome(homeDir, () -> {
+                try {
+                    Registry registry = Registry.Builder.builder()
+                            .defaults("myuser", "mypass")
+                            .build();
+
+                    ContainerRef containerSource = ContainerRef.parse("the-source");
+                    Path file1 = blobDir.resolve("source.txt");
+                    Files.writeString(file1, "foobar");
+
+                    // Push
+                    Manifest manifest = registry.pushArtifact(containerSource, LocalPath.of(file1));
+                    assertNotNull(manifest);
+
+                    // Copy to other registry
+                    ContainerRef containerTarget = ContainerRef.parse("the-target");
+                    CopyUtils.copy(registry, containerSource, registry, containerTarget, false);
+                    registry.pullArtifact(containerTarget, artifactDir, true);
+                    assertEquals("foobar", Files.readString(artifactDir.resolve("source.txt")));
+                } catch (Exception e) {
+                    throw new RuntimeException(e);
+                }
+            });
+        }
+    }
+
     @Test
     void testShouldCopyFromOciLayoutToRegistryNonRecursive() throws IOException {