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

Updating package via bosh vendor-package --prefix does not honor prefix on dependent packages #606

Closed
jrussett opened this issue Feb 2, 2023 · 3 comments
Closed

Updating package via bosh vendor-package --prefix does not honor prefix on dependent packages #606

jrussett opened this issue Feb 2, 2023 · 3 comments
Assignees
@rkoster

Comments

@jrussett
Copy link
Contributor

jrussett commented Feb 2, 2023 •
edited

Summary

When you update a package via bosh vendor-package --prefix and the desired package has dependent packages, the dependent packages are not updated with a prefix.

Note

On the first invocation of bosh vendor-package with --prefix, the desired package and its dependent packages will all be vendored with a prefix. This error only occurs when re-vendoring the package to update it.

Steps to Reproduce

For this specific example, we will use the healthchecker package, which requires the golang-1-linux package to compile the binary. The healthchecker package is intended to be vendored into releases like silk, cf-networking, and garden-runc.

When vendoring another bosh package with a PREFIX that depends one of the golang-*-linux packages, the golang-*-linux will be vendored with prefix (e.g. PREFIX-golang-*-linux).

If I run the following

cd ~/workspace/silk-release
bosh vendor-package healthchecker ~/workspace/healthchecker-release --prefix silk

Then I end up with the following packages:

silk-healthchecker/
silk-golang-1-linux/

Where silk-healthchecker/spec.lock looks something like:

name: silk-healthchecker
fingerprint: 11fec354b909c9d6463e9edd3c92bb7c01423eb0df21c2f7ebe742c5d6f34e28
dependencies:
- silk-golang-1-linux

Running bosh vendor-package --prefix again to update the package

Now, when I want to update the healthchecker package, I see that the silk-golang-1-linux dependency is renamed, removing the prefix entirely!

diff --git a/packages/silk-golang-1-linux/spec.lock b/packages/silk-golang-1-linux/spec.lock
 name: silk-golang-1-linux
-fingerprint: b55bf89806a951e9b3812e987bd526ef931550ab01360f002b38e00d83f8296b
+fingerprint: db9120088b5e3ad5c6f52742a8f1c21a9b234a3b79cf916d48b22660c7ac422e

diff --git a/packages/silk-healthchecker/spec.lock b/packages/silk-healthchecker/spec.lock
 name: silk-healthchecker
-fingerprint: 11fec354b909c9d6463e9edd3c92bb7c01423eb0df21c2f7ebe742c5d6f34e28
+fingerprint: d823a8fec682820aa8e56d115e3d0aa15c309e2120dc183854b80e5e69fb3dcf
 dependencies:
-- silk-golang-1-linux
+- golang-1-linux

Expected Results

When I'm updating the healthchecker package via bosh vendor-package --prefix, only the fingerprints indicating the updated package should change. The names of the dependencies should stay the same:

diff --git a/packages/silk-golang-1-linux/spec.lock b/packages/silk-golang-1-linux/spec.lock
 name: silk-golang-1-linux
-fingerprint: b55bf89806a951e9b3812e987bd526ef931550ab01360f002b38e00d83f8296b
+fingerprint: db9120088b5e3ad5c6f52742a8f1c21a9b234a3b79cf916d48b22660c7ac422e

diff --git a/packages/silk-healthchecker/spec.lock b/packages/silk-healthchecker/spec.lock
 name: silk-healthchecker
-fingerprint: 11fec354b909c9d6463e9edd3c92bb7c01423eb0df21c2f7ebe742c5d6f34e28
+fingerprint: d823a8fec682820aa8e56d115e3d0aa15c309e2120dc183854b80e5e69fb3dcf

Additional Text Output, Screenshots, contextual information (optional)

Adding --prefix to bosh vendor-package: #603
@ramonskie
Copy link
Contributor

i never tested this with a package that also had a dependency.
i will look in to it.

@rkoster rkoster self-assigned this Feb 9, 2023
nouseforaname pushed a commit that referenced this issue Feb 14, 2023
fix inconsistent vendor package behaviour
a1ba11d 
when we introduced the prefixed vendored package behaviour we overlooked
a case that could lead to inconstistent writes of the spec.lock for a
prefixed package.

The issue was described in #606

The underlying cause was the ordering of `allInterestingPkgs`
https://github.com/cloudfoundry/bosh-cli/pull/603/files#diff-420d09437c4bf08b4407d39b5956c6209df3519cde36b121a856085e372d2c92R240-R241

As setup in the added test, we have 3 packages. The top level package
`pkg-1` and the two dependencies it pulls in `pkg-2,pkg-3`

The ordering of the map `allInterestingPkgs` is inconsistent and the
test will fail for every case where pkg-1 isn't the last item in
`allInterestingPkgs`. That is because we end up calling
writeVendoredPackage on pkg-1, which in turn will iterate over the
dependencies.

At this point we didn't yet call .Prefix and .Finalize on the objects
for `pkg-2` and `pkg-3`. Though this is required so they can return
their prefixed name via Name().

This is addressed by duplicating the loop. The first pass will ensure
all objects in the dependency tree are prefixed and finalized before
the spec.lock is written to disk.
nouseforaname pushed a commit that referenced this issue Feb 14, 2023
fix inconsistent vendor package behaviour
be78cc3 
when we introduced the prefixed vendored package behaviour we overlooked
a case that could lead to inconstistent writes of the spec.lock for a
prefixed package.

The issue was described in #606

The underlying cause was the ordering of `allInterestingPkgs`
https://github.com/cloudfoundry/bosh-cli/pull/603/files#diff-420d09437c4bf08b4407d39b5956c6209df3519cde36b121a856085e372d2c92R240-R241

As setup in the added test, we have 3 packages. The top level package
`pkg-1` and the two dependencies it pulls in `pkg-2,pkg-3`

The ordering of the map `allInterestingPkgs` is inconsistent and the
test will fail for every case where pkg-1 isn't the last item in
`allInterestingPkgs`. That is because we end up calling
writeVendoredPackage on pkg-1, which in turn will iterate over the
dependencies.

At this point we didn't yet call .Prefix and .Finalize on the objects
for `pkg-2` and `pkg-3`. Though this is required so they can return
their prefixed name via Name().

This is addressed by duplicating the loop. The first pass will ensure
all objects in the dependency tree are prefixed and finalized before
the spec.lock is written to disk.
@nouseforaname nouseforaname mentioned this issue Feb 14, 2023
Merged
@jpalermo
Copy link
Member

Hey @jrussett, can you try v7.1.3 and see if it fixes the problem?

@jrussett
Copy link
Contributor Author

jrussett commented Feb 21, 2023 •
edited

Hey @jpalermo

I just tested this in our CI after confirming our docker image had the new bosh cli. The cli was able to successfully vendor the package with a prefix without clobbering the dependencies! ✅
cloudfoundry/garden-runc-release@46efe91

Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rkoster rkoster

Labels
None yet
Projects
Foundational Infrastructure Working Group
Archived in project
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jpalermo @rkoster @ramonskie @jrussett