Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes mirror bug when originalHost is excluded #569

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Fixes mirror bug when originalHost is excluded #569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions postal/internal/dependency_mirror.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ func formatAndVerifyMirror(mirror, uri string) (string, error) {
return "", err
}

if mirrorURL.Scheme != "https" && mirrorURL.Scheme != "file" {
if strings.ToLower(mirrorURL.Scheme) != "https" && strings.ToLower(mirrorURL.Scheme) != "file" {
return "", fmt.Errorf("invalid mirror scheme")
}

mirrorURL.Path = strings.Replace(mirrorURL.Path, "{originalHost}", uriURL.Host+uriURL.Path, 1)
mirrorURL.Path = strings.Replace(mirrorURL.Path, "{originalHost}", uriURL.Hostname(), 1) + uriURL.Path
return mirrorURL.String(), nil
}

Expand Down
70 changes: 45 additions & 25 deletions postal/internal/dependency_mirror_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {

context("given a default mirror binding", func() {
it("finds a matching dependency mirror in the platform bindings if there is one", func() {
boundDependency, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(bindingResolver.ResolveCall.Receives.Typ).To(Equal("dependency-mirror"))
Expect(bindingResolver.ResolveCall.Receives.Provider).To(BeEmpty())
Expect(bindingResolver.ResolveCall.Receives.PlatformDir).To(Equal("some-platform-dir"))
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri"))
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri.com/dep.tgz"))
})
})

Expand All @@ -73,21 +73,21 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
})

it("finds the default mirror when given uri does not match a specific hostname", func() {
boundDependency, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(bindingResolver.ResolveCall.Receives.Typ).To(Equal("dependency-mirror"))
Expect(bindingResolver.ResolveCall.Receives.Provider).To(BeEmpty())
Expect(bindingResolver.ResolveCall.Receives.PlatformDir).To(Equal("some-platform-dir"))
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri"))
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri.com/dep.tgz"))
})

it("finds the mirror matching the specific hostname in the given uri", func() {
boundDependency, err := resolver.FindDependencyMirror("some-git.luolix.top-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-git.luolix.top-uri/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(bindingResolver.ResolveCall.Receives.Typ).To(Equal("dependency-mirror"))
Expect(bindingResolver.ResolveCall.Receives.Provider).To(BeEmpty())
Expect(bindingResolver.ResolveCall.Receives.PlatformDir).To(Equal("some-platform-dir"))
Expect(boundDependency).To(Equal("https://mirror.example.org/public-github"))
Expect(boundDependency).To(Equal("https://mirror.example.org/public-github/dep.tgz"))
})
})

Expand All @@ -104,7 +104,7 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
})

it("return empty string for non specific hostnames", func() {
boundDependency, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(bindingResolver.ResolveCall.Receives.Typ).To(Equal("dependency-mirror"))
Expect(bindingResolver.ResolveCall.Receives.Provider).To(BeEmpty())
Expand All @@ -113,12 +113,12 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
})

it("finds the mirror matching the specific hostname in the given uri", func() {
boundDependency, err := resolver.FindDependencyMirror("some-nodejs.org-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-nodejs.org-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(bindingResolver.ResolveCall.Receives.Typ).To(Equal("dependency-mirror"))
Expect(bindingResolver.ResolveCall.Receives.Provider).To(BeEmpty())
Expect(bindingResolver.ResolveCall.Receives.PlatformDir).To(Equal("some-platform-dir"))
Expect(boundDependency).To(Equal("https://mirror.example.org/node-dist"))
Expect(boundDependency).To(Equal("https://mirror.example.org/node-dist/dep.tgz"))
})
})
})
Expand All @@ -137,9 +137,9 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {

context("given the default mirror environment variable is set", func() {
it("finds the matching dependency mirror", func() {
boundDependency, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri"))
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri.com/dep.tgz"))
})
})

Expand All @@ -155,21 +155,21 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
})

it("finds the default mirror when given uri does not match a specific hostname", func() {
boundDependency, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri"))
Expect(boundDependency).To(Equal("https://mirror.example.org/some-uri.com/dep.tgz"))
})

it("finds the mirror matching the specific hostname in the given uri", func() {
boundDependency, err := resolver.FindDependencyMirror("some-git.luolix.top-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-git.luolix.top-uri/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).To(Equal("https://mirror.example.org/public-github"))
Expect(boundDependency).To(Equal("https://mirror.example.org/public-github/dep.tgz"))
})

it("properly decodes the hostname", func() {
boundDependency, err := resolver.FindDependencyMirror("testing.123-abc-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://testing.123-abc-uri/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).To(Equal("https://mirror.example.org/testing"))
Expect(boundDependency).To(Equal("https://mirror.example.org/testing/dep.tgz"))
})
})

Expand All @@ -184,15 +184,15 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
})

it("return empty string for non specific hostnames", func() {
boundDependency, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).To(Equal(""))
})

it("finds the mirror matching the specific hostname in the given uri", func() {
boundDependency, err := resolver.FindDependencyMirror("some-git.luolix.top-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-git.luolix.top-uri/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).To(Equal("https://mirror.example.org/public-github"))
Expect(boundDependency).To(Equal("https://mirror.example.org/public-github/dep.tgz"))
})
})
})
Expand All @@ -213,14 +213,34 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
})

it("defaults to environment variable and ignores binding", func() {
boundDependency, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).NotTo(Equal("https://mirror.example.org/some-uri"))
Expect(boundDependency).To(Equal("https://mirror.other-example.org/some-uri"))
Expect(boundDependency).NotTo(Equal("https://mirror.example.org/some-uri.com/dep.tgz"))
Expect(boundDependency).To(Equal("https://mirror.other-example.org/some-uri.com/dep.tgz"))

})
})

context("without originalHost placeholder", func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on what the bug that you're solving is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user can use {originalHost} as a placeholder to include the original url path when specifying a mirror. For example, if the original url is https://artifacts.paketo.io/nginx_1.24.0_linux_x64_jammy_fa1dfe46.tgz and the mirror is https://user:pass@local-mirror.example.com/buildpacks-dependencies/{originalHost}, then this will translate to https://user:pass@local-mirror.example.com/buildpacks-dependencies/artifacts.paketo.io/nginx_1.24.0_linux_x64_jammy_fa1dfe46.tgz. But if the placeholder is excluded, then the mirror translates to https://user:pass@local-mirror.example.com/buildpacks-dependencies/nginx_1.24.0_linux_x64_jammy_fa1dfe46.tgz

it.Before(func() {
Expect(os.Setenv("BP_DEPENDENCY_MIRROR", "https://mirror.example.org"))

bindingResolver = &fakes.BindingResolver{}
resolver = internal.NewDependencyMirrorResolver(bindingResolver)
})

it.After(func() {
Expect(os.Unsetenv("BP_DEPENDENCY_MIRROR"))
})

it("sets mirror excluding original hostname", func() {
boundDependency, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).ToNot(HaveOccurred())
Expect(boundDependency).To(Equal("https://mirror.example.org/dep.tgz"))
})

})

context("failure cases", func() {
context("when more than one dependency mirror binding exists", func() {
it.Before(func() {
Expand Down Expand Up @@ -258,7 +278,7 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
},
}

_, err = resolver.FindDependencyMirror("some-uri", "some-platform-dir")
_, err = resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).To(MatchError(ContainSubstring("cannot have multiple bindings of type 'dependency-mirror'")))
})
})
Expand All @@ -276,7 +296,7 @@ func testDependencyMirror(t *testing.T, context spec.G, it spec.S) {
})

it("returns an error", func() {
_, err := resolver.FindDependencyMirror("some-uri", "some-platform-dir")
_, err := resolver.FindDependencyMirror("https://some-uri.com/dep.tgz", "some-platform-dir")
Expect(err).To(MatchError(ContainSubstring("invalid mirror scheme")))
})
})
Expand Down
Loading