pivotal-cf / kiln

Kiln helps you maintain product tiles for VMware Tanzu Operations Manager.
Apache License 2.0
30 stars 20 forks source link

Fix/artifactory url normalization #472

Closed notrepo05 closed 9 months ago

notrepo05 commented 10 months ago

We were blocked from using kiln with other Artifactory servers, in our case development servers, due to find-release-version and update-release-version behaving differently from fetch for artifactory release sources.

The issue was found in internal/component/artifactory.go DownloadRelease where /artifactory is always appended to the server url:

downloadURL := ars.ArtifactoryHost + "/artifactory/" + ars.Repo + "/" + remoteRelease.RemotePath

find-release fails if we don't include /artifactory in the Kilnfile artifactory_host variable

failing

➜  hello-tile git:(feat/artifactory-source) ✗ :workspace/hello-tile$ kiln find-release-version --release hello-release --no-download -vr artifactory_username="admin" -vr artifactory_password="password" -vr artifactory_host="localhost:8082"
2024/01/23 10:31:03 could not execute "find-release-version": not found

working

➜  hello-tile git:(feat/artifactory-source) ✗ :workspace/hello-tile$ kiln find-release-version --release hello-release --no-download -vr artifactory_username="admin" -vr artifactory_password="password" -vr artifactory_host="localhost:8082/artifactory"
[Artifactory release source] 2024/01/23 10:31:07 Getting hello-release file info from artifactory
{"version":"0.2.5","remote_path":"compiled-releases/hello-release/hello-release-0.2.5-ubuntu-jammy-1.260.tgz","source":"artifactory","sha":"2c40c91568fd98a815c246916aa2b8ee80bce8cd"}

fetch, however automatically appends /artifactory, causing 404s due to requests containing /artifactory/artifactory:

failing

➜  hello-tile git:(feat/artifactory-source) ✗ :workspace/hello-tile$ kiln fetch -vr artifactory_username="admin" -vr artifactory_password="password" -vr artifactory_host="localhost:8082/artifactory"
Warning: The "allow-only-publishable-releases" flag was not set. Some fetched releases may be intended for development/testing only. EXERCISE CAUTION WHEN PUBLISHING A TILE WITH THESE RELEASES!
Gathering releases...
Found 2 missing releases to download
[Artifactory release source] 2024/01/23 10:31:36 downloading hello-release from artifactory release source artifactory
2024/01/23 10:31:36 could not execute "fetch": download failed: error from release source "artifactory": failed to download hello-release release from artifactory with error code 404

working

➜  hello-tile git:(feat/artifactory-source) ✗ :workspace/hello-tile$ kiln fetch -vr artifactory_username="admin" -vr artifactory_password="password" -vr artifactory_host="localhost:8082"
Warning: The "allow-only-publishable-releases" flag was not set. Some fetched releases may be intended for development/testing only. EXERCISE CAUTION WHEN PUBLISHING A TILE WITH THESE RELEASES!
Gathering releases...
Found 2 missing releases to download
[Artifactory release source] 2024/01/23 10:32:29 downloading hello-release from artifactory release source artifactory
[Artifactory release source] 2024/01/23 10:32:30 downloading bpm from artifactory release source artifactory
...

Thank you

cf-gitbot commented 10 months ago

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

notrepo05 commented 10 months ago
➜  kiln git:(fix/artifactory-url-normalization) ✗ :workspace/kiln$ go test ./...
?       github.com/pivotal-cf/kiln  [no test files]
?       github.com/pivotal-cf/kiln/internal/baking/fakes    [no test files]
?       github.com/pivotal-cf/kiln/internal/builder/fakes   [no test files]
?       github.com/pivotal-cf/kiln/internal/commands/fakes  [no test files]
?       github.com/pivotal-cf/kiln/internal/component/fakes [no test files]
?       github.com/pivotal-cf/kiln/internal/component/fakes_internal    [no test files]
?       github.com/pivotal-cf/kiln/internal/test/fakes  [no test files]
?       github.com/pivotal-cf/kiln/internal/test-helpers    [no test files]
?       github.com/pivotal-cf/kiln/pkg/notes/internal/fakes [no test files]
?       github.com/pivotal-cf/kiln/pkg/planitest/internal/fakes [no test files]
ok      github.com/pivotal-cf/kiln/internal/acceptance/bake 4.727s
ok      github.com/pivotal-cf/kiln/internal/acceptance/workflows    (cached)
ok      github.com/pivotal-cf/kiln/internal/acceptance/workflows/scenario   2.230s
ok      github.com/pivotal-cf/kiln/internal/baking  (cached)
ok      github.com/pivotal-cf/kiln/internal/builder (cached)
ok      github.com/pivotal-cf/kiln/internal/commands    1.839s
ok      github.com/pivotal-cf/kiln/internal/commands/flags  (cached)
ok      github.com/pivotal-cf/kiln/internal/component   1.172s
ok      github.com/pivotal-cf/kiln/internal/gh  (cached)
ok      github.com/pivotal-cf/kiln/internal/helper  (cached)
ok      github.com/pivotal-cf/kiln/internal/pivnet  (cached)
ok      github.com/pivotal-cf/kiln/internal/test    38.943s
ok      github.com/pivotal-cf/kiln/pkg/cargo    (cached)
ok      github.com/pivotal-cf/kiln/pkg/history  (cached)
ok      github.com/pivotal-cf/kiln/pkg/notes    (cached)
ok      github.com/pivotal-cf/kiln/pkg/planitest    (cached)
ok      github.com/pivotal-cf/kiln/pkg/planitest/internal   (cached)
ok      github.com/pivotal-cf/kiln/pkg/proofing (cached)
ok      github.com/pivotal-cf/kiln/pkg/proofing/upgrade (cached)
ok      github.com/pivotal-cf/kiln/pkg/source   (cached)
ok      github.com/pivotal-cf/kiln/pkg/tile (cached)
crhntr commented 9 months ago

I re-created the PR from a different branch (on this repository not a fork) https://github.com/pivotal-cf/kiln/pull/477