Skip to content

Commit 8b729db

Browse files
authored
feat: don't preserve extra container repository path from origin (#341)
* feat: don't preserve extra container repository path from origin Signed-off-by: Tomas Pizarro Moreno <tomas.pizarro@broadcom.com> * fix lint and tests Signed-off-by: Tomas Pizarro Moreno <tomas.pizarro@broadcom.com> --------- Signed-off-by: Tomas Pizarro Moreno <tomas.pizarro@broadcom.com>
1 parent 3bcc1e9 commit 8b729db

5 files changed

Lines changed: 131 additions & 5 deletions

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ require (
1717
github.com/spf13/cobra v1.10.2
1818
github.com/spf13/viper v1.20.1
1919
github.com/stretchr/testify v1.11.1
20-
github.com/vmware-labs/distribution-tooling-for-helm v0.6.1
20+
github.com/vmware-labs/distribution-tooling-for-helm v0.7.0
2121
google.golang.org/protobuf v1.36.11
2222
gopkg.in/yaml.v3 v3.0.1
2323
helm.sh/helm/v3 v3.19.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -689,8 +689,8 @@ github.com/transparency-dev/merkle v0.0.2 h1:Q9nBoQcZcgPamMkGn7ghV8XiTZ/kRxn1yCG
689689
github.com/transparency-dev/merkle v0.0.2/go.mod h1:pqSy+OXefQ1EDUVmAJ8MUhHB9TXGuzVAT58PqBoHz1A=
690690
github.com/vbatts/tar-split v0.12.1 h1:CqKoORW7BUWBe7UL/iqTVvkTBOF8UvOMKOIZykxnnbo=
691691
github.com/vbatts/tar-split v0.12.1/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA=
692-
github.com/vmware-labs/distribution-tooling-for-helm v0.6.1 h1:dlhWvDevr3P86B7BipkREhMOtSZS9WxNoAwLtRUSewY=
693-
github.com/vmware-labs/distribution-tooling-for-helm v0.6.1/go.mod h1:Q6WupfRDDMi6qBcta2IK6UUAt14xe64ug8kLRfiuImY=
692+
github.com/vmware-labs/distribution-tooling-for-helm v0.7.0 h1:FJXG066yrovW8U/mEgQbhtQZe11DS0zpIot+GGVQzGA=
693+
github.com/vmware-labs/distribution-tooling-for-helm v0.7.0/go.mod h1:Q6WupfRDDMi6qBcta2IK6UUAt14xe64ug8kLRfiuImY=
694694
github.com/vmware-labs/yaml-jsonpath v0.3.2 h1:/5QKeCBGdsInyDCyVNLbXyilb61MXGi9NP674f9Hobk=
695695
github.com/vmware-labs/yaml-jsonpath v0.3.2/go.mod h1:U6whw1z03QyqgWdgXxvVnQ90zN1BWz5V+51Ewf8k+rQ=
696696
github.com/vmware-tanzu/carvel-imgpkg v0.38.3 h1:vVnqCPFEZ2NQcoTywg/va91qRyCuu46wBYAETqoyez4=

pkg/client/target/common/common.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ func NewContainer(target *apiv1.Target, containersReaderWriter client.Containers
6868
func (t *Target) getContainersUploadURL() string {
6969
containersURL := t.containersURL
7070
if containersURL == "" {
71-
containersURL = t.GetUploadURL()
71+
// When containers URL is not specified, append /containers to charts URL
72+
// to avoid collision between charts and containers at the same path
73+
containersURL = t.GetUploadURL() + "/containers"
7274
}
7375

7476
if schemeRE.MatchString(containersURL) {
@@ -97,6 +99,7 @@ func (t *Target) UnwrapChart(file string, _ *chart.Metadata, opts ...config.Opti
9799
unwrap.WithContainerRegistryAuth(t.containersUsername, t.containersPassword),
98100
unwrap.WithSkipImageRelocation(cfg.SkipImages),
99101
unwrap.WithSkipPullImages(cfg.SkipImages),
102+
unwrap.WithPreserveRepository(false),
100103
); err != nil {
101104
return errors.Trace(err)
102105
}
@@ -124,6 +127,7 @@ func (t *Target) UnwrapContainer(file string, opts ...config.Option) error {
124127
unwrap.WithSkipImageRelocation(cfg.SkipImages),
125128
unwrap.WithSkipPullImages(cfg.SkipImages),
126129
unwrap.WithFetchArtifacts(!cfg.SkipArtifacts),
130+
unwrap.WithPreserveRepository(false),
127131
); err != nil {
128132
return errors.Trace(err)
129133
}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package common
2+
3+
import (
4+
"testing"
5+
6+
apiv1 "github.com/bitnami/charts-syncer/gen/proto/v1"
7+
"github.com/bitnami/charts-syncer/pkg/client/types"
8+
)
9+
10+
// MockChartsReaderWriter is a mock implementation of ChartsReaderWriter
11+
type MockChartsReaderWriter struct {
12+
uploadURL string
13+
}
14+
15+
func (m *MockChartsReaderWriter) Fetch(_, _ string) (string, error) {
16+
return "", nil
17+
}
18+
19+
func (m *MockChartsReaderWriter) List() ([]string, error) {
20+
return []string{}, nil
21+
}
22+
23+
func (m *MockChartsReaderWriter) ListChartVersions(_ string) ([]string, error) {
24+
return []string{}, nil
25+
}
26+
27+
func (m *MockChartsReaderWriter) Has(_, _ string) (bool, error) {
28+
return false, nil
29+
}
30+
31+
func (m *MockChartsReaderWriter) GetChartDetails(_, _ string) (*types.ChartDetails, error) {
32+
return nil, nil
33+
}
34+
35+
func (m *MockChartsReaderWriter) GetUploadURL() string {
36+
return m.uploadURL
37+
}
38+
39+
func TestGetContainersUploadURL_WithoutExplicitContainersURL(t *testing.T) {
40+
// Test case: target.containers is nil, should append /containers to chart URL
41+
target := &apiv1.Target{
42+
Repo: &apiv1.Repo{
43+
Url: "http://127.0.0.1:5000/library-replicated/charts/photon-5",
44+
},
45+
Containers: nil,
46+
}
47+
48+
t.Run("containers_url_not_specified", func(t *testing.T) {
49+
mockCharts := &MockChartsReaderWriter{
50+
uploadURL: "http://127.0.0.1:5000/library-replicated/charts/photon-5",
51+
}
52+
targetObj, err := New(target, mockCharts, false, false)
53+
if err != nil {
54+
t.Fatalf("error creating target: %v", err)
55+
}
56+
57+
got := targetObj.getContainersUploadURL()
58+
want := "127.0.0.1:5000/library-replicated/charts/photon-5/containers"
59+
60+
if got != want {
61+
t.Errorf("got %q, want %q", got, want)
62+
}
63+
})
64+
}
65+
66+
func TestGetContainersUploadURL_WithExplicitContainersURL(t *testing.T) {
67+
// Test case: target.containers is specified, should use that URL
68+
target := &apiv1.Target{
69+
Repo: &apiv1.Repo{
70+
Url: "http://127.0.0.1:5000/library-replicated/charts/photon-5",
71+
},
72+
Containers: &apiv1.Containers{
73+
Url: "http://127.0.0.1:5000/library-replicated/containers/photon-5",
74+
},
75+
}
76+
77+
t.Run("containers_url_specified", func(t *testing.T) {
78+
mockCharts := &MockChartsReaderWriter{
79+
uploadURL: "http://127.0.0.1:5000/library-replicated/charts/photon-5",
80+
}
81+
targetObj, err := New(target, mockCharts, false, false)
82+
if err != nil {
83+
t.Fatalf("error creating target: %v", err)
84+
}
85+
86+
got := targetObj.getContainersUploadURL()
87+
want := "127.0.0.1:5000/library-replicated/containers/photon-5"
88+
89+
if got != want {
90+
t.Errorf("got %q, want %q", got, want)
91+
}
92+
})
93+
}
94+
95+
func TestGetContainersUploadURL_StripsScheme(t *testing.T) {
96+
// Test case: verify that scheme is stripped correctly
97+
target := &apiv1.Target{
98+
Repo: &apiv1.Repo{
99+
Url: "oci://127.0.0.1:5000/library-replicated/charts/photon-5",
100+
},
101+
Containers: nil,
102+
}
103+
104+
t.Run("scheme_stripped_with_containers_suffix", func(t *testing.T) {
105+
mockCharts := &MockChartsReaderWriter{
106+
uploadURL: "oci://127.0.0.1:5000/library-replicated/charts/photon-5",
107+
}
108+
targetObj, err := New(target, mockCharts, false, false)
109+
if err != nil {
110+
t.Fatalf("error creating target: %v", err)
111+
}
112+
113+
got := targetObj.getContainersUploadURL()
114+
// Scheme should be stripped, and /containers should be appended before scheme removal
115+
// Actually, the scheme removal happens after appending /containers
116+
want := "127.0.0.1:5000/library-replicated/charts/photon-5/containers"
117+
118+
if got != want {
119+
t.Errorf("got %q, want %q", got, want)
120+
}
121+
})
122+
}

test/run-verifications.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ set -o pipefail
66

77
# Constants
88
FAILED_TEST=0
9-
EXPECTED_REGISTRY='localhost:5000/library/bitnami'
9+
EXPECTED_REGISTRY='localhost:5000/library/containers'
1010

1111
## Check that WordPress deployment is using the expected registry
1212
wordpressImage=$(kubectl get pods --selector=app.kubernetes.io/name=wordpress -ojsonpath='{.items[0].spec.containers[0].image}')

0 commit comments

Comments
 (0)