This repository has been archived by the owner on Jun 13, 2021. It is now read-only.
Compose validation #760
Merged
Compose validation #760
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
Codecov Report
@@ Coverage Diff @@
## master #760 +/- ##
=========================================
+ Coverage 70.07% 71.27% +1.2%
=========================================
Files 64 67 +3
Lines 3519 3837 +318
=========================================
+ Hits 2466 2735 +269
- Misses 727 768 +41
- Partials 326 334 +8
Continue to review full report at Codecov.
|
| for _, p := range m { | ||
| str, ok := p.(string) | ||
| if !ok { | ||
| errs = append(errs, fmt.Errorf("invalid volume in service %q", s.service)) |
There was a problem hiding this comment.
I wonder if those checks could be removed, if we validate the compose file first using the json schema.
There was a problem hiding this comment.
Once we have schema validation then yeah, sure
silvin-lubecki
approved these changes
Nov 21, 2019
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Remove the no longer used check for the relative path Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
This check is done by the packager now Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
The map implementation of go iterates a map random;y so we can't know which error has what message. It's ok to remove this here because we have an e2e test (the final errors are sorted alphabetically). Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
rumpl
force-pushed
the
compose-validation
branch
from
1b17130
to
25bc6b4
Compare
A secret *must* be external, not *should* Signed-off-by: Djordje Lukic <lukic.djordje@gmail.com>
eunomie
reviewed
Nov 25, 2019
It can have more than one `:` in its definition. Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 I did
Created a "framework" to help write validation rules for a compose file, the
Validatorwill do the heavy lifting of traversing all the properties in the compose file.The validator works in two phases:
relativepathruleneeds the top levelvolumesto check if the volume exists.A rule has 3 parts:
The
Collectmethod is called in the first phase, rules collect dependent data if needed here.The
Acceptmethod is used to filter out only thing that a rule needs to validateThe
Validatemethod does the actual validation and returns a list of errors for a given value.The PR introduces two rules:
externalsecretruleto force all secrets to be externalrelativepathruleto ban relative paths in volumesIn a followup we should:
validatecommandI won't do them here since the changeset is starting to get big.
- How to verify it
Unit and e2e tests were added for the two rules
- A picture of a cute animal (not mandatory)
