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.

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

add BOSH Release tarball reading utilities #410

Merged
merged 7 commits into from
Jul 20, 2023

Conversation

crhntr
Copy link
Contributor

@crhntr crhntr commented Jul 5, 2023

This adds some utilities for reading BOSH release manifests (release.MF) from tarballs.

I refactor the builder and acceptance tests to use this to show that this makes sense as a place for these utility functions. My main intent for adding them was to use them in the kiln buildpacks.

@cf-gitbot
Copy link
Member

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.

@crhntr crhntr changed the title Add more bosh release metadata utilities Add BOSH Release tarball reading utilities Jul 6, 2023
Copy link
Contributor

@jaristiz jaristiz left a comment

Choose a reason for hiding this comment

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

Like the refactor to the release package, found it confusing when using github.com/kiln/pkg/tile package in other projects.

@crhntr crhntr force-pushed the add-more-bosh-release-metadata-utilities branch from 3a597de to ac9600c Compare July 6, 2023 22:16
@crhntr
Copy link
Contributor Author

crhntr commented Jul 6, 2023

I agree renaming it would make it make more sense. I also don't like that I take the "tile" identifier; it is such a good variable name. Maybe tilerelease? Unfortunately, "release" is such an overloaded term.

@dtimm
Copy link
Contributor

dtimm commented Jul 7, 2023

Why add this to kiln specifically? I think it would make more sense as a separate repository that is used in kiln rather than included wholesale. Since it isn't used (yet), could the PR wait until the functionality is needed?

@crhntr
Copy link
Contributor Author

crhntr commented Jul 7, 2023

We do use these utilities in kiln bake.

I have found these utilities to be useful in buildpacks and so I wanted to put these utilities somewhere open source. They used to be in PPET but I have found using that even for internal projects not easy. GOPRIVATE is not fun to set up in CI.

After thinking about your comment. I think these functions make more sense in cargo because they operate on the BOSH Release Tarballs within the tiles. Also it might make sense to add the bosh release package data to the Kilnfile.lock.

@crhntr crhntr force-pushed the add-more-bosh-release-metadata-utilities branch from 9fa2b1d to d1ea71a Compare July 7, 2023 17:08
- refator: add test coverage for TestReadBOSHReleaseManifestsFromTarballs
- feat: add Stemcell method on BOSHReleaseManifest
- fix linter errors
- refactor: use new stemcell utility
@crhntr crhntr force-pushed the add-more-bosh-release-metadata-utilities branch from b2b8b89 to 3fa56ea Compare July 13, 2023 22:02
@crhntr crhntr changed the title Add BOSH Release tarball reading utilities add BOSH Release tarball reading utilities Jul 14, 2023
this reduces number of templating functions in the kilnfile.
interpolation in the Kilnfile is a mistake and should be factored out

this is to remove circular dependency when we move bosh release tarball functions to cargo
this is easier to read
@crhntr crhntr force-pushed the add-more-bosh-release-metadata-utilities branch 2 times, most recently from e11a673 to dbe7c82 Compare July 14, 2023 21:31
@crhntr crhntr force-pushed the add-more-bosh-release-metadata-utilities branch from dbe7c82 to 1d1254b Compare July 14, 2023 21:36
@crhntr crhntr merged commit df1320c into main Jul 20, 2023
@crhntr crhntr deleted the add-more-bosh-release-metadata-utilities branch July 20, 2023 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants