fix(container): handle image refs with port numbers and clean up temp tags
- Fix normalizeImageRef to correctly parse localhost:5000/image:tag - The last colon before a tag has no slash after it - Add cleanup of temporary localhost:5000 tags after canonical tagging - Add test for normalizeImageRef with port numbers
- commit
- 757fa05f346d9bab4c81b3386709b9c8a155e4fb
- author
- brice · · 2026-01-14 06:23 UTC
- parent
- 118d20e
- signature
- unsigned
2 files · +48 -3
M internal/usecase/container/service.go +33 -3
| @@ -509,6 +509,14 @@ func (s *Service) ensureImage(ctx context.Context, imageRef string) (string, err | |||
| 509 | 509 | return "", err | |
| 510 | 510 | } | |
| 511 | 511 | ||
| 512 | + | // Clean up the temporary pull reference tag to avoid duplicate entries | |
| 513 | + | if pullRef != imageRef { | |
| 514 | + | if err := s.runtime.UntagImage(ctx, pullRef); err != nil { | |
| 515 | + | // Log but don't fail - the canonical tag is already applied | |
| 516 | + | log.Debug().Err(err).Str("pull_ref", pullRef).Msg("failed to remove temporary pull tag") | |
| 517 | + | } | |
| 518 | + | } | |
| 519 | + | ||
| 512 | 520 | log.Info().Msg("image pulled successfully") | |
| 513 | 521 | return imageRef, nil | |
| 514 | 522 | } | |
| @@ -534,6 +542,12 @@ func (s *Service) ensureLocalImage(ctx context.Context, imageRef, pullRef string | |||
| 534 | 542 | if err := s.tagImageIfNeeded(ctx, pullRef, imageRef); err != nil { | |
| 535 | 543 | return false, err | |
| 536 | 544 | } | |
| 545 | + | // Clean up the temporary pull reference tag | |
| 546 | + | if pullRef != imageRef { | |
| 547 | + | if err := s.runtime.UntagImage(ctx, pullRef); err != nil { | |
| 548 | + | log.Debug().Err(err).Str("pull_ref", pullRef).Msg("failed to remove temporary pull tag") | |
| 549 | + | } | |
| 550 | + | } | |
| 537 | 551 | log.Info().Msg("image found locally, skipping pull") | |
| 538 | 552 | return true, nil | |
| 539 | 553 | } | |
| @@ -890,14 +904,30 @@ func (s *Service) stopAllManagedContainers(ctx context.Context) error { | |||
| 890 | 904 | // Utility functions | |
| 891 | 905 | ||
| 892 | 906 | func normalizeImageRef(image string) string { | |
| 893 | - | parts := strings.Split(image, ":") | |
| 894 | - | repo := parts[0] | |
| 907 | + | // Extract repository from image reference, handling port numbers correctly. | |
| 908 | + | // Examples: | |
| 909 | + | // nginx:latest -> docker.io/library/nginx | |
| 910 | + | // user/repo:tag -> docker.io/user/repo | |
| 911 | + | // localhost:5000/image:tag -> localhost:5000/image | |
| 912 | + | // registry.example.com/image:tag -> registry.example.com/image | |
| 913 | + | ||
| 914 | + | // Find the tag separator: the last colon that isn't part of a port number. | |
| 915 | + | // A colon is part of a port if there's no slash after it until the next colon. | |
| 916 | + | repo := image | |
| 917 | + | lastColon := strings.LastIndex(image, ":") | |
| 918 | + | if lastColon != -1 { | |
| 919 | + | afterColon := image[lastColon+1:] | |
| 920 | + | // If there's no slash after the colon, it's the tag separator | |
| 921 | + | if !strings.Contains(afterColon, "/") { | |
| 922 | + | repo = image[:lastColon] | |
| 923 | + | } | |
| 924 | + | } | |
| 895 | 925 | ||
| 896 | 926 | if !strings.Contains(repo, "/") { | |
| 897 | 927 | return "docker.io/library/" + repo | |
| 898 | 928 | } | |
| 899 | 929 | ||
| 900 | - | if strings.Count(repo, "/") == 1 && !strings.Contains(strings.Split(repo, "/")[0], ".") { | |
| 930 | + | if strings.Count(repo, "/") == 1 && !strings.Contains(strings.Split(repo, "/")[0], ".") && !strings.Contains(strings.Split(repo, "/")[0], ":") { | |
| 901 | 931 | return "docker.io/" + repo | |
| 902 | 932 | } | |
| 903 | 933 | ||
| @@ -509,6 +509,14 @@ func (s *Service) ensureImage(ctx context.Context, imageRef string) (string, err | |||||
| 509 | return "", err | 509 | return "", err | ||
| 510 | } | 510 | } | ||
| 511 | 511 | ||||
| 512 | + | // Clean up the temporary pull reference tag to avoid duplicate entries | |||
| 513 | + | if pullRef != imageRef { | |||
| 514 | + | if err := s.runtime.UntagImage(ctx, pullRef); err != nil { | |||
| 515 | + | // Log but don't fail - the canonical tag is already applied | |||
| 516 | + | log.Debug().Err(err).Str("pull_ref", pullRef).Msg("failed to remove temporary pull tag") | |||
| 517 | + | } | |||
| 518 | + | } | |||
| 519 | + | ||||
| 512 | log.Info().Msg("image pulled successfully") | 520 | log.Info().Msg("image pulled successfully") | ||
| 513 | return imageRef, nil | 521 | return imageRef, nil | ||
| 514 | } | 522 | } | ||
| @@ -534,6 +542,12 @@ func (s *Service) ensureLocalImage(ctx context.Context, imageRef, pullRef string | |||||
| 534 | if err := s.tagImageIfNeeded(ctx, pullRef, imageRef); err != nil { | 542 | if err := s.tagImageIfNeeded(ctx, pullRef, imageRef); err != nil { | ||
| 535 | return false, err | 543 | return false, err | ||
| 536 | } | 544 | } | ||
| 545 | + | // Clean up the temporary pull reference tag | |||
| 546 | + | if pullRef != imageRef { | |||
| 547 | + | if err := s.runtime.UntagImage(ctx, pullRef); err != nil { | |||
| 548 | + | log.Debug().Err(err).Str("pull_ref", pullRef).Msg("failed to remove temporary pull tag") | |||
| 549 | + | } | |||
| 550 | + | } | |||
| 537 | log.Info().Msg("image found locally, skipping pull") | 551 | log.Info().Msg("image found locally, skipping pull") | ||
| 538 | return true, nil | 552 | return true, nil | ||
| 539 | } | 553 | } | ||
| @@ -890,14 +904,30 @@ func (s *Service) stopAllManagedContainers(ctx context.Context) error { | |||||
| 890 | // Utility functions | 904 | // Utility functions | ||
| 891 | 905 | ||||
| 892 | func normalizeImageRef(image string) string { | 906 | func normalizeImageRef(image string) string { | ||
| 893 | - | parts := strings.Split(image, ":") | 907 | + | // Extract repository from image reference, handling port numbers correctly. |
| 894 | - | repo := parts[0] | 908 | + | // Examples: |
| 909 | + | // nginx:latest -> docker.io/library/nginx | |||
| 910 | + | // user/repo:tag -> docker.io/user/repo | |||
| 911 | + | // localhost:5000/image:tag -> localhost:5000/image | |||
| 912 | + | // registry.example.com/image:tag -> registry.example.com/image | |||
| 913 | + | ||||
| 914 | + | // Find the tag separator: the last colon that isn't part of a port number. | |||
| 915 | + | // A colon is part of a port if there's no slash after it until the next colon. | |||
| 916 | + | repo := image | |||
| 917 | + | lastColon := strings.LastIndex(image, ":") | |||
| 918 | + | if lastColon != -1 { | |||
| 919 | + | afterColon := image[lastColon+1:] | |||
| 920 | + | // If there's no slash after the colon, it's the tag separator | |||
| 921 | + | if !strings.Contains(afterColon, "/") { | |||
| 922 | + | repo = image[:lastColon] | |||
| 923 | + | } | |||
| 924 | + | } | |||
| 895 | 925 | ||||
| 896 | if !strings.Contains(repo, "/") { | 926 | if !strings.Contains(repo, "/") { | ||
| 897 | return "docker.io/library/" + repo | 927 | return "docker.io/library/" + repo | ||
| 898 | } | 928 | } | ||
| 899 | 929 | ||||
| 900 | - | if strings.Count(repo, "/") == 1 && !strings.Contains(strings.Split(repo, "/")[0], ".") { | 930 | + | if strings.Count(repo, "/") == 1 && !strings.Contains(strings.Split(repo, "/")[0], ".") && !strings.Contains(strings.Split(repo, "/")[0], ":") { |
| 901 | return "docker.io/" + repo | 931 | return "docker.io/" + repo | ||
| 902 | } | 932 | } | ||
| 903 | 933 | ||||
M internal/usecase/container/service_test.go +15 -0
| @@ -556,6 +556,21 @@ func TestNormalizeImageRef(t *testing.T) { | |||
| 556 | 556 | input: "nginx:latest", | |
| 557 | 557 | expected: "docker.io/library/nginx", | |
| 558 | 558 | }, | |
| 559 | + | { | |
| 560 | + | name: "localhost with port", | |
| 561 | + | input: "localhost:5000/myapp:latest", | |
| 562 | + | expected: "localhost:5000/myapp", | |
| 563 | + | }, | |
| 564 | + | { | |
| 565 | + | name: "localhost with port no tag", | |
| 566 | + | input: "localhost:5000/myapp", | |
| 567 | + | expected: "localhost:5000/myapp", | |
| 568 | + | }, | |
| 569 | + | { | |
| 570 | + | name: "registry domain with port", | |
| 571 | + | input: "registry.example.com:5000/image:v1.0", | |
| 572 | + | expected: "registry.example.com:5000/image", | |
| 573 | + | }, | |
| 559 | 574 | } | |
| 560 | 575 | ||
| 561 | 576 | for _, tt := range tests { | |
| @@ -556,6 +556,21 @@ func TestNormalizeImageRef(t *testing.T) { | |||||
| 556 | input: "nginx:latest", | 556 | input: "nginx:latest", | ||
| 557 | expected: "docker.io/library/nginx", | 557 | expected: "docker.io/library/nginx", | ||
| 558 | }, | 558 | }, | ||
| 559 | + | { | |||
| 560 | + | name: "localhost with port", | |||
| 561 | + | input: "localhost:5000/myapp:latest", | |||
| 562 | + | expected: "localhost:5000/myapp", | |||
| 563 | + | }, | |||
| 564 | + | { | |||
| 565 | + | name: "localhost with port no tag", | |||
| 566 | + | input: "localhost:5000/myapp", | |||
| 567 | + | expected: "localhost:5000/myapp", | |||
| 568 | + | }, | |||
| 569 | + | { | |||
| 570 | + | name: "registry domain with port", | |||
| 571 | + | input: "registry.example.com:5000/image:v1.0", | |||
| 572 | + | expected: "registry.example.com:5000/image", | |||
| 573 | + | }, | |||
| 559 | } | 574 | } | ||
| 560 | 575 | ||||
| 561 | for _, tt := range tests { | 576 | for _, tt := range tests { | ||