fix(container): prevent double-prefixing images with localhost registry
buildImageRef() used strings.Split(image, ":")[0] to extract the repo part, which breaks when RegistryDomain contains a port (localhost:5000). This caused images to be double-prefixed as localhost:5000/localhost:5000/myapp. Fix by checking if image already starts with the registry domain prefix directly, and using smarter tag extraction that handles port numbers.
- commit
- c031e6ffe1a4abd4c07e57940e2ec3d1a2757a1d
- author
- bnema · · 2026-02-03 19:47 UTC
- parent
- ef62f35
- signature
- unsigned
2 files · +88 -2
M internal/usecase/container/service.go +22 -2
| @@ -636,10 +636,30 @@ func (s *Service) buildImageRef(image string) string { | |||
| 636 | 636 | return image | |
| 637 | 637 | } | |
| 638 | 638 | ||
| 639 | - | repoPart := strings.Split(image, ":")[0] | |
| 640 | - | if strings.HasPrefix(repoPart, s.config.RegistryDomain+"/") { | |
| 639 | + | // Check if image already has the registry domain prefix | |
| 640 | + | prefix := s.config.RegistryDomain + "/" | |
| 641 | + | if strings.HasPrefix(image, prefix) { | |
| 641 | 642 | return image | |
| 642 | 643 | } | |
| 644 | + | ||
| 645 | + | // Check if image is from an external registry (has dot and slash in the repo part). | |
| 646 | + | // Extract repo part by finding tag separator (last colon not in digest). | |
| 647 | + | // For "docker.io/library/nginx:latest" -> "docker.io/library/nginx" | |
| 648 | + | // For "myapp:latest" -> "myapp" | |
| 649 | + | // For "localhost:5000/myapp:latest" -> "localhost:5000/myapp" (already handled above) | |
| 650 | + | repoPart := image | |
| 651 | + | if idx := strings.LastIndex(image, ":"); idx != -1 { | |
| 652 | + | // Check if this colon is a tag separator (not part of a digest or port) | |
| 653 | + | afterColon := image[idx+1:] | |
| 654 | + | if !strings.HasPrefix(afterColon, "sha256:") && !strings.Contains(afterColon, "/") { | |
| 655 | + | repoPart = image[:idx] | |
| 656 | + | } | |
| 657 | + | } | |
| 658 | + | // Handle digest references: repo@sha256:... | |
| 659 | + | if idx := strings.Index(repoPart, "@"); idx != -1 { | |
| 660 | + | repoPart = repoPart[:idx] | |
| 661 | + | } | |
| 662 | + | ||
| 643 | 663 | if strings.Contains(repoPart, ".") && strings.Contains(repoPart, "/") { | |
| 644 | 664 | return image | |
| 645 | 665 | } | |
| @@ -636,10 +636,30 @@ func (s *Service) buildImageRef(image string) string { | |||||
| 636 | return image | 636 | return image | ||
| 637 | } | 637 | } | ||
| 638 | 638 | ||||
| 639 | - | repoPart := strings.Split(image, ":")[0] | 639 | + | // Check if image already has the registry domain prefix |
| 640 | - | if strings.HasPrefix(repoPart, s.config.RegistryDomain+"/") { | 640 | + | prefix := s.config.RegistryDomain + "/" |
| 641 | + | if strings.HasPrefix(image, prefix) { | |||
| 641 | return image | 642 | return image | ||
| 642 | } | 643 | } | ||
| 644 | + | ||||
| 645 | + | // Check if image is from an external registry (has dot and slash in the repo part). | |||
| 646 | + | // Extract repo part by finding tag separator (last colon not in digest). | |||
| 647 | + | // For "docker.io/library/nginx:latest" -> "docker.io/library/nginx" | |||
| 648 | + | // For "myapp:latest" -> "myapp" | |||
| 649 | + | // For "localhost:5000/myapp:latest" -> "localhost:5000/myapp" (already handled above) | |||
| 650 | + | repoPart := image | |||
| 651 | + | if idx := strings.LastIndex(image, ":"); idx != -1 { | |||
| 652 | + | // Check if this colon is a tag separator (not part of a digest or port) | |||
| 653 | + | afterColon := image[idx+1:] | |||
| 654 | + | if !strings.HasPrefix(afterColon, "sha256:") && !strings.Contains(afterColon, "/") { | |||
| 655 | + | repoPart = image[:idx] | |||
| 656 | + | } | |||
| 657 | + | } | |||
| 658 | + | // Handle digest references: repo@sha256:... | |||
| 659 | + | if idx := strings.Index(repoPart, "@"); idx != -1 { | |||
| 660 | + | repoPart = repoPart[:idx] | |||
| 661 | + | } | |||
| 662 | + | ||||
| 643 | if strings.Contains(repoPart, ".") && strings.Contains(repoPart, "/") { | 663 | if strings.Contains(repoPart, ".") && strings.Contains(repoPart, "/") { | ||
| 644 | return image | 664 | return image | ||
| 645 | } | 665 | } | ||
M internal/usecase/container/service_test.go +66 -0
| @@ -743,6 +743,72 @@ func TestMergeEnvironmentVariables(t *testing.T) { | |||
| 743 | 743 | assert.Contains(t, result, "BAZ=user") | |
| 744 | 744 | } | |
| 745 | 745 | ||
| 746 | + | func TestBuildImageRef(t *testing.T) { | |
| 747 | + | tests := []struct { | |
| 748 | + | name string | |
| 749 | + | image string | |
| 750 | + | registryAuthEnabled bool | |
| 751 | + | registryDomain string | |
| 752 | + | wantRef string | |
| 753 | + | }{ | |
| 754 | + | { | |
| 755 | + | name: "adds registry domain prefix", | |
| 756 | + | image: "myapp:latest", | |
| 757 | + | registryAuthEnabled: true, | |
| 758 | + | registryDomain: "registry.example.com", | |
| 759 | + | wantRef: "registry.example.com/myapp:latest", | |
| 760 | + | }, | |
| 761 | + | { | |
| 762 | + | name: "keeps existing registry domain prefix", | |
| 763 | + | image: "registry.example.com/myapp:latest", | |
| 764 | + | registryAuthEnabled: true, | |
| 765 | + | registryDomain: "registry.example.com", | |
| 766 | + | wantRef: "registry.example.com/myapp:latest", | |
| 767 | + | }, | |
| 768 | + | { | |
| 769 | + | name: "skips external registry images", | |
| 770 | + | image: "docker.io/library/nginx:latest", | |
| 771 | + | registryAuthEnabled: true, | |
| 772 | + | registryDomain: "registry.example.com", | |
| 773 | + | wantRef: "docker.io/library/nginx:latest", | |
| 774 | + | }, | |
| 775 | + | { | |
| 776 | + | name: "returns original when auth disabled", | |
| 777 | + | image: "myapp:latest", | |
| 778 | + | registryAuthEnabled: false, | |
| 779 | + | registryDomain: "registry.example.com", | |
| 780 | + | wantRef: "myapp:latest", | |
| 781 | + | }, | |
| 782 | + | { | |
| 783 | + | name: "localhost domain adds prefix", | |
| 784 | + | image: "myapp:latest", | |
| 785 | + | registryAuthEnabled: true, | |
| 786 | + | registryDomain: "localhost:5000", | |
| 787 | + | wantRef: "localhost:5000/myapp:latest", | |
| 788 | + | }, | |
| 789 | + | { | |
| 790 | + | name: "localhost domain keeps existing prefix", | |
| 791 | + | image: "localhost:5000/myapp:latest", | |
| 792 | + | registryAuthEnabled: true, | |
| 793 | + | registryDomain: "localhost:5000", | |
| 794 | + | wantRef: "localhost:5000/myapp:latest", // BUG: currently returns localhost:5000/localhost:5000/myapp:latest | |
| 795 | + | }, | |
| 796 | + | } | |
| 797 | + | ||
| 798 | + | for _, tt := range tests { | |
| 799 | + | t.Run(tt.name, func(t *testing.T) { | |
| 800 | + | svc := &Service{ | |
| 801 | + | config: Config{ | |
| 802 | + | RegistryAuthEnabled: tt.registryAuthEnabled, | |
| 803 | + | RegistryDomain: tt.registryDomain, | |
| 804 | + | }, | |
| 805 | + | } | |
| 806 | + | gotRef := svc.buildImageRef(tt.image) | |
| 807 | + | assert.Equal(t, tt.wantRef, gotRef, "unexpected image reference") | |
| 808 | + | }) | |
| 809 | + | } | |
| 810 | + | } | |
| 811 | + | ||
| 746 | 812 | func TestRewriteToRegistryDomain(t *testing.T) { | |
| 747 | 813 | tests := []struct { | |
| 748 | 814 | name string | |
| @@ -743,6 +743,72 @@ func TestMergeEnvironmentVariables(t *testing.T) { | |||||
| 743 | assert.Contains(t, result, "BAZ=user") | 743 | assert.Contains(t, result, "BAZ=user") | ||
| 744 | } | 744 | } | ||
| 745 | 745 | ||||
| 746 | + | func TestBuildImageRef(t *testing.T) { | |||
| 747 | + | tests := []struct { | |||
| 748 | + | name string | |||
| 749 | + | image string | |||
| 750 | + | registryAuthEnabled bool | |||
| 751 | + | registryDomain string | |||
| 752 | + | wantRef string | |||
| 753 | + | }{ | |||
| 754 | + | { | |||
| 755 | + | name: "adds registry domain prefix", | |||
| 756 | + | image: "myapp:latest", | |||
| 757 | + | registryAuthEnabled: true, | |||
| 758 | + | registryDomain: "registry.example.com", | |||
| 759 | + | wantRef: "registry.example.com/myapp:latest", | |||
| 760 | + | }, | |||
| 761 | + | { | |||
| 762 | + | name: "keeps existing registry domain prefix", | |||
| 763 | + | image: "registry.example.com/myapp:latest", | |||
| 764 | + | registryAuthEnabled: true, | |||
| 765 | + | registryDomain: "registry.example.com", | |||
| 766 | + | wantRef: "registry.example.com/myapp:latest", | |||
| 767 | + | }, | |||
| 768 | + | { | |||
| 769 | + | name: "skips external registry images", | |||
| 770 | + | image: "docker.io/library/nginx:latest", | |||
| 771 | + | registryAuthEnabled: true, | |||
| 772 | + | registryDomain: "registry.example.com", | |||
| 773 | + | wantRef: "docker.io/library/nginx:latest", | |||
| 774 | + | }, | |||
| 775 | + | { | |||
| 776 | + | name: "returns original when auth disabled", | |||
| 777 | + | image: "myapp:latest", | |||
| 778 | + | registryAuthEnabled: false, | |||
| 779 | + | registryDomain: "registry.example.com", | |||
| 780 | + | wantRef: "myapp:latest", | |||
| 781 | + | }, | |||
| 782 | + | { | |||
| 783 | + | name: "localhost domain adds prefix", | |||
| 784 | + | image: "myapp:latest", | |||
| 785 | + | registryAuthEnabled: true, | |||
| 786 | + | registryDomain: "localhost:5000", | |||
| 787 | + | wantRef: "localhost:5000/myapp:latest", | |||
| 788 | + | }, | |||
| 789 | + | { | |||
| 790 | + | name: "localhost domain keeps existing prefix", | |||
| 791 | + | image: "localhost:5000/myapp:latest", | |||
| 792 | + | registryAuthEnabled: true, | |||
| 793 | + | registryDomain: "localhost:5000", | |||
| 794 | + | wantRef: "localhost:5000/myapp:latest", // BUG: currently returns localhost:5000/localhost:5000/myapp:latest | |||
| 795 | + | }, | |||
| 796 | + | } | |||
| 797 | + | ||||
| 798 | + | for _, tt := range tests { | |||
| 799 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 800 | + | svc := &Service{ | |||
| 801 | + | config: Config{ | |||
| 802 | + | RegistryAuthEnabled: tt.registryAuthEnabled, | |||
| 803 | + | RegistryDomain: tt.registryDomain, | |||
| 804 | + | }, | |||
| 805 | + | } | |||
| 806 | + | gotRef := svc.buildImageRef(tt.image) | |||
| 807 | + | assert.Equal(t, tt.wantRef, gotRef, "unexpected image reference") | |||
| 808 | + | }) | |||
| 809 | + | } | |||
| 810 | + | } | |||
| 811 | + | ||||
| 746 | func TestRewriteToRegistryDomain(t *testing.T) { | 812 | func TestRewriteToRegistryDomain(t *testing.T) { | ||
| 747 | tests := []struct { | 813 | tests := []struct { | ||
| 748 | name string | 814 | name string | ||