Sanitize archive folder name #2154
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Yingrong Zhao <[email protected]>
carolynvs
reviewed
Jun 15, 2022
Signed-off-by: Yingrong Zhao <[email protected]>
carolynvs
requested changes
Jun 15, 2022
pkg/porter/archive_test.go
Outdated
| dir, err := ex.createArchiveFolder("examples/test-bundle-0.2.0") | ||
| require.NoError(t, err) | ||
|
|
||
| info, err := ex.fs.Stat(dir) |
There was a problem hiding this comment.
Can you add one more assert that checks the name created? That will help check for replacing the path seperator with a dash.
There was a problem hiding this comment.
Great catch! I added it
carolynvs
reviewed
Jun 15, 2022
Signed-off-by: Yingrong Zhao <[email protected]>
joshuabezaleel
pushed a commit
to joshuabezaleel/porter
that referenced
this pull request
Jun 23, 2022
* fix archive folder creation Signed-off-by: Yingrong Zhao <[email protected]> * replace path separator instead Signed-off-by: Yingrong Zhao <[email protected]> * modify test Signed-off-by: Yingrong Zhao <[email protected]> Signed-off-by: joshuabezaleel <[email protected]>
carolynvs
added a commit
that referenced
this pull request
Jun 23, 2022
* Use user specified directory for resolving file path (#2142) * use user specified build directory if provided for porter manifest Signed-off-by: Yingrong Zhao <[email protected]> * update tests Signed-off-by: Yingrong Zhao <[email protected]> * update doc and fix tests Signed-off-by: Yingrong Zhao <[email protected]> * address comment Signed-off-by: Yingrong Zhao <[email protected]> * explicitly set default value for o.Dir Signed-off-by: Yingrong Zhao <[email protected]> * clearer help text Signed-off-by: Yingrong Zhao <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Update to helm3 mixin v0.1.16 v0.1.16 includes fixes for using nonroot invocation images Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Sanitize archive folder name (#2154) * fix archive folder creation Signed-off-by: Yingrong Zhao <[email protected]> * replace path separator instead Signed-off-by: Yingrong Zhao <[email protected]> * modify test Signed-off-by: Yingrong Zhao <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Adding pagination for installation, parameter, and credential list result using skip and limit option (#2137) * Add pagination option for installation list command using skip and limit flag Signed-off-by: joshuabezaleel <[email protected]> * Increase plugin start/stop timeouts As I was adding back in net/rpc plugins (the legacy v0 plugins), I realized that our plugin timeouts don't work well for net/rpc since it is much slower than gRPC. I've bumped both the plugin start and stop timeout defaults to make it less likely that a user will run into the timeout, while still giving us a good "oops the plugin is broken" timeout detection. Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Add InstallationStore.FindInstallations (#2119) The advanced dependencies proposal needs to be able to search for installations based on more complex critieria than is available in the ListInstallations function (which is intended to support the porter installation list command). FindInstallations lets us craft any valid mongodb find query and execute it, returning a list of installations. Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Rename DisplayRun.ClaimID to ID I missed this field when I did a sweep earlier to remove the use of the word claim in the release/v1 branch. In the rest of the CLI's output we call the run's id just ID or RunID, and should be consistent with that. I've changed DisplayID.ClaimID to ID so that we aren't exposing the term claim to our users (and it's not really the claim id anymore anyway). Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Support Docker TLS environment variables We are using the docker cli library to build images and I had thought this gave us automatic support for building against a remote docker host. It works fine for DOCKER_HOST, but turns out the TLS configuration environment variables are only parsed when the docker CLI flags are bound (which doesn't occur when we use it as a library). I've updated how we initialize the docker cli library so that DOCKER_TLS_VERIFY and DOCKER_CERT_PATH are picked up and passed to the library. Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Add vet and lint targets to magefile Signed-off-by: Tanmay Chaudhry <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Add ListOption input parameter struct and enable skip and limit option to credential and parameter list command as well Signed-off-by: joshuabezaleel <[email protected]> * Leave out default value for ListOption's properties Signed-off-by: joshuabezaleel <[email protected]> * Remove commented function signature Signed-off-by: joshuabezaleel <[email protected]> * Convert CreateListFilter to ToFindOptions method for ListOptions type receiver Signed-off-by: joshuabezaleel <[email protected]> Co-authored-by: Carolyn Van Slyck <[email protected]> Co-authored-by: Tanmay Chaudhry <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Add state and status to list installation Signed-off-by: joshuabezaleel <[email protected]> * fix archive folder test Signed-off-by: Yingrong Zhao <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Fix Vet Errors (#2153) * Fix lint errors for unkeyed fields in composite literals Signed-off-by: Tanmay Chaudhry <[email protected]> * resolve lint errors on tags Signed-off-by: Tanmay Chaudhry <[email protected]> * Updated golden file to account for bad struct tag fix Signed-off-by: Tanmay Chaudhry <[email protected]> * Vet Fix: Rename example tests to use suffixes. Signed-off-by: Tanmay Chaudhry <[email protected]> * Replace ExtendedBundle{} initialization with a NewBundle constructor Signed-off-by: Tanmay Chaudhry <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Improve error message loading wrong schema (#2157) * Improve error message loading wrong schema Signed-off-by: Kevin Barbour <[email protected]> * Add myself to CONTRIBUTORS.MD Signed-off-by: Kevin Barbour <[email protected]> * Don't use errors pkg, fix assert in test Signed-off-by: Kevin Barbour <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Add prow github action This adds a prow github action that allows specified people (in the OWNERS file) to comment on a pull request with /lgtm to review the pull request, or /approve to merge the pull request. The github action handles executing the commands for you so that you don't need to have maintainer rights on the repository. Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Switch prow to use pull_request instead of _target Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Updated installation schema with correct dependency schema Signed-off-by: Steven Gettys <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * changed new manifest description for test Signed-off-by: Steven Gettys <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Update k8s and containerd dependencies * Update to cnab-go v0.23.4 * Update containerd to v1.6.6 * Updated k8s to v0.24.1. This does not update docker since buildkit uses a funny unreleased version of docker. We won't be able to update to a new version of Docker until there's a release that has the new feature that buildkit relies upon. See go.mod for a link to the troublesome package in question. Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: joshuabezaleel <[email protected]> * Add comments Signed-off-by: joshuabezaleel <[email protected]> * StateDefined as default value Signed-off-by: joshuabezaleel <[email protected]> * Move displayinstallation's state and status to metadata Signed-off-by: joshuabezaleel <[email protected]> * Add golden file test for print installation Signed-off-by: joshuabezaleel <[email protected]> * Add unit test for displayInstallation's state and status Signed-off-by: joshuabezaleel <[email protected]> * Change function name from set to get Signed-off-by: joshuabezaleel <[email protected]> * Revert changes on test file Signed-off-by: joshuabezaleel <[email protected]> * add new line Signed-off-by: joshuabezaleel <[email protected]> * resolve conflict Signed-off-by: joshuabezaleel <[email protected]> * fix comment Signed-off-by: joshuabezaleel <[email protected]> Co-authored-by: Yingrong Zhao <[email protected]> Co-authored-by: Carolyn Van Slyck <[email protected]> Co-authored-by: Tanmay Chaudhry <[email protected]> Co-authored-by: Kevin Barbour <[email protected]> Co-authored-by: Steven Gettys <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change
Currently, porter uses bundle name as part of the temporary archive folder for storing all artifacts and metadata that's needed for archiving a bundle.
The bundle name may contain invalid path characters, which causes
porter archiveto fail.This PR fixes above issue by sanitizing bundle name to remove non-standard path characters before using it as part of the archive folder path.
If a path separator is present in a bundle name, we will replace them with a dash,
-What issue does it fix
Closes #2151
Notes for the reviewer
Checklist
Reviewer Checklist