add local testing via tilt and remove typhon#47
Closed
DaytonG wants to merge 1 commit into
Closed
Conversation
- add tilt testing
Author
|
Is there anyone I can get to review this? |
|
Hi @DaytonG, appreciate the PR. Could you break it into 2 PRs, one for the tilt/local dev changes and the typhon/vendor folder changes in this one? I'd like to hold off on the tilt changes until I can familiarize myself with that tool, never used it. The golang changes I don't have any issues with, I can merge that in today or tomorrow after taking a closer look and running tests. Thanks! |
Author
|
Made a new PR, but I propose we keep this one open for the tilt changes, as it is a superset of #48 |
|
Hello, I am leaving the Redbox org and will close this PR as I won't have time to review and it is several months old already |
Closed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
I realize this is a large, opinionated change. So feel free to reject this.
I work for a company that has strict security requirements. Our security scanners detected CVE-2020-14040 within scuttle, which looks to be caused by an indirect dependency through the
typhonpackage. Looking at the code, I noticed that typhon could basically be replaced with the builtinnet/httppackage, which is part of this PR. This has the added benefit of reducing the footprint of scuttle itself (11.6M down to 6.4M) while also decreasing its vulnerability footprint (slightly).Since I was rooting around, I also nuked the vendor directory. Since this project uses modules effectively, I figured it was not as necessary.
Thirdly, I integrated KIND+tilt local development, as the docker-compose seemed to be broken. I've had great success with tilt in the past, and it allows us all to use a few simple commands to test our changes in real-deal kubernetes Pods.
Lastly, I fixed a few warnings surrounding uncaught errors (by just... ignoring most of them).