Refactoring Docker URI Parser (#4448)

* refactor: sha256 and tag parser refactored. Digest's hash value moved to function parse_docker_uri

* fix: double-quated converted to single-quoted

* refactor: splitting tag and digest from path refactored

* fix: rubocop suggestions are corrected

Author: serdarozerr

lib/cloud_controller/diego/docker/docker_uri_converter.rb

@@ -5,14 +5,12 @@ class DockerURIConverter
     def convert(docker_uri)
       raise UriUtils::InvalidDockerURI.new "Docker URI [#{docker_uri}] should not contain scheme" if docker_uri.include? '://'
 
-      host, path, tag = UriUtils.parse_docker_uri(docker_uri)
+      host, path, tag_digest = UriUtils.parse_docker_uri(docker_uri)
 
-      if !tag.nil? && tag.start_with?('@sha256:')
-        path = "#{path}@sha256"
-        tag.slice!('@sha256:')
-      end
-
-      Addressable::URI.new(scheme: 'docker', host: host, path: path, fragment: tag).to_s
+      # add tag or digest part as fragment to the uri, since ruby uri parser confuses with ':'
+      # when it presented in path. We convert user's uri to, for example;
+      # docker://docker.io/publish/ubuntu:latest -> docker://docker.io/publish/ubuntu#latest
+      Addressable::URI.new(scheme: 'docker', host: host, path: path, fragment: tag_digest).to_s
     end
   end
 end

lib/utils/uri_utils.rb

@@ -4,6 +4,11 @@ module UriUtils
   SSH_REGEX = %r{ \A (?:ssh://)? git@ .+? : .+? \.git \z }x
   GIT_REGEX = %r{ \A git:// .+? : .+? \.git \z }x
   DOCKER_INDEX_SERVER = 'docker.io'.freeze
+  DOCKER_PATH_REGEX = %r{\A[a-z0-9_\-\.\/]{2,255}\Z}
+  DOCKER_TAG_REGEX = /[a-zA-Z0-9_\-\.]{1,128}/
+  DOCKER_DIGEST_REGEX = /sha256:[a-z0-9]{64}/
+  DOCKER_TAG_DIGEST_REGEX = Regexp.new("\\A(#{DOCKER_TAG_REGEX.source} |
+(#{DOCKER_TAG_REGEX.source}@#{DOCKER_DIGEST_REGEX.source}) | #{DOCKER_DIGEST_REGEX.source})\\Z", Regexp::EXTENDED)
 
   class InvalidDockerURI < StandardError; end
 
@@ -62,13 +67,20 @@ def self.parse_docker_uri(docker_uri)
     end
 
     path = 'library/' + path if (official_docker_registry(name_parts[0]) || missing_registry(name_parts)) && path.exclude?('/')
+    path, tag_digest = parse_docker_tag_digest_from_path(path)
 
-    path, tag = parse_docker_repository_tag(path)
+    raise InvalidDockerURI.new "Invalid image name [#{path}]" unless DOCKER_PATH_REGEX =~ path
+    raise InvalidDockerURI.new "Invalid image tag [#{tag_digest}]" if tag_digest && DOCKER_TAG_DIGEST_REGEX !~ tag_digest
 
-    raise InvalidDockerURI.new "Invalid image name [#{path}]" unless %r{\A[a-z0-9_\-\.\/]{2,255}\Z} =~ path
-    raise InvalidDockerURI.new "Invalid image tag [#{tag}]" if tag && !(/\A(([a-zA-Z0-9_\-\.]{1,128})|(([a-zA-Z0-9_\-\.]{0,128})(@sha256:[a-z0-9]{64})))\Z/ =~ tag)
+    # if only sha256 presented, we add hash value as fragment to the uri,
+    # since the ruby uri parser confuses because of second ':' in uri's path part.
+    if tag_digest && tag_digest.start_with?('sha256:')
+      _, hash_value = tag_digest.split(':')
+      path += '@sha256'
+      tag_digest = hash_value
+    end
 
-    [host, path, tag]
+    [host, path, tag_digest]
   end
 
   private_class_method def self.official_docker_registry(host)
@@ -81,11 +93,29 @@ def self.parse_docker_uri(docker_uri)
     (host.exclude?('.') && host.exclude?(':') && host != 'localhost')
   end
 
-  private_class_method def self.parse_docker_repository_tag(path)
-    path, tag = path.split(/(?=@)|:/, 2)
+  private_class_method def self.parse_docker_tag_digest_from_path(path)
+    # Split path into base path and digest if digest is present (after '@')
+    base_path, digest = path.split('@', 2)
+
+    if digest
+      # If digest is present and base_path contains a tag (':'), split it
+      if base_path.include?(':')
+        base_path, tag = base_path.split(':', 2)
+        # Return path and combined tag@digest
+        return [base_path, "#{tag}@#{digest}"]
+      end
+
+      # Return path and digest if no tag present
+      return [base_path, digest]
+    end
+
+    # No digest present, check for tag
+    base_path, tag = base_path.split(':', 2)
 
-    return [path, tag] unless tag && tag.include?('/')
+    # If tag is present but looks like a path segment (contains '/'), treat as no tag
+    return [base_path, 'latest'] if tag&.include?('/')
 
-    [path, 'latest']
+    # Return path and tag (or nil if no tag)
+    [base_path, tag]
   end
 end

spec/unit/lib/utils/uri_utils_spec.rb

@@ -159,7 +159,7 @@
       expect(UriUtils.parse_docker_uri('publish/buildpack:tag')).to eq ['', 'publish/buildpack', 'tag']
 
       actual_result = UriUtils.parse_docker_uri('publish/buildpack@sha256:e118d023acaee5cf13471ead39f68416ad6172ff0899f3257ce1481cd2b28a6a')
-      expected_result = ['', 'publish/buildpack', '@sha256:e118d023acaee5cf13471ead39f68416ad6172ff0899f3257ce1481cd2b28a6a']
+      expected_result = ['', 'publish/buildpack@sha256', 'e118d023acaee5cf13471ead39f68416ad6172ff0899f3257ce1481cd2b28a6a']
       expect(actual_result).to eq expected_result
 
       actual_result = UriUtils.parse_docker_uri('publish/buildpack:tag@sha256:e118d023acaee5cf13471ead39f68416ad6172ff0899f3257ce1481cd2b28a6a')