feat: Remove magic strings from Source type #77
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
|
The source variants, with all sub-fields, have been added to the `Source` struct. Note that this code does not yet work, and will not compile. Signed-off-by: Peter Engelbert <[email protected]>
These are basic getters and setters for the `Ref` subfields on various `Source` variants. Because any one of these variants could be non-nil, we first have to determine which is in use. Not all Source vaiants have a `.Ref` field, so those cases need to be handled as well. These methods each return an additional `bool` value depending on whether or not the `.Ref` field exists in the Source variant. Signed-off-by: Peter Engelbert <[email protected]>
Change switch on scheme to union variant in `source2LLBGetter` (`source.go`). Signed-off-by: Peter Engelbert <[email protected]>
Continue implementing the necessary switches. This code compiles. Signed-off-by: Peter Engelbert <[email protected]>
This field is now found on `Source.Git`. Signed-off-by: Peter Engelbert <[email protected]>
Also, remove `Source.Source` and `SourceAnotherSource`. Signed-off-by: Peter Engelbert <[email protected]>
* Remove source.GetRef and source.SetRef * Add source.processArgs and use it where necessary Signed-off-by: Peter Engelbert <[email protected]>
It's often convenient to use files and directories from the local filesystem as a source. This commit essentially exposes `llb.Local(...)` to the user via the spec. This implementation may be incomplete, and should be reviewed. Signed-off-by: Peter Engelbert <[email protected]>
Validate that a source does not have multiple variants defined, and that it has at least one variant defined. If there's a more elegant implementation, feel free to suggest it. Signed-off-by: Peter Engelbert <[email protected]>
It may be necessary in the future to allow specification of commands on `Build` and `Context` source variants. For now, it is only possible for Docker images. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
The format has changed, and this check is no longer correct. This commit also re-runs the `go generate` command and commits the changes. Signed-off-by: Peter Engelbert <[email protected]>
Note: Only some of the tests will run properly. The logic for `SourceBuild` needs to be re-tooled in order to run one of the tests properly. I will come back later to edit the rest of the test fixtures. Signed-off-by: Peter Engelbert <[email protected]>
`SourceBuild` needs a base state of some sort, in which the specified Dockerfile will be executed. This implementation allows the base state to be made up of a local path, a build context, or another source. This may not be final, and it's likely that there are mistakes in here. Signed-off-by: Peter Engelbert <[email protected]>
The tests are still not completing successfully, so more troubleshooting is needed. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
After discussion with Brian, this is the correct struct layout for this. The context string is a path which will be used as the `.` context for the dockerfile build. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
To pass the linter Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Because of the patching PR. Signed-off-by: Peter Engelbert <[email protected]>
We can no longer reference other sources. Instead, we can specify a `local` source with the current directory in order to build the frontend. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Additionally, ensure that the defaults are filled correctly for the `context` source type (which made the `local` type redundant). Signed-off-by: Peter Engelbert <[email protected]>
We're no longer using the schemes Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Use them instead of pointers. Also, validate them up-front in the `validate` function, so that they do not need to be checked later. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Also, validate sources recursively. Signed-off-by: Peter Engelbert <[email protected]>
Also: - update `processArgs` to do substitutions inline. - update `fillDefaults` to set the default Source.Path to `"."` when the Source type is Context. Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Clearer to do it with some code repetition Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Ensure the commit specified in the spec makes it to the call to `llb.Git`. Signed-off-by: Peter Engelbert <[email protected]>
The unit test revealed some bugs, which have been fixed. Also, validation error messages have been improved. Signed-off-by: Peter Engelbert <[email protected]>
pmengelbert
force-pushed
the
pmengelbert/remove_magic_strings/4
branch
from
4ea0dca to
97a52f5
Compare
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
Signed-off-by: Peter Engelbert <[email protected]>
cpuguy83
reviewed
Jan 23, 2024
spec.go
Outdated
| // === Begin Source Variants === | ||
| DockerImage *SourceDockerImage `yaml:"image,omitempty" json:"image,omitempty"` | ||
| Git *SourceGit `yaml:"git,omitempty" json:"git,omitempty"` | ||
| HTTPS *SourceHTTPS `yaml:"https,omitempty" json:"https,omitempty"` |
There was a problem hiding this comment.
Nit: this should probably just be http.
Tried to think of a better name than that but am coming up short.
There was a problem hiding this comment.
The field name? The Struct name? The json tag? All of the above?
Signed-off-by: Peter Engelbert <[email protected]>
See previous commit Signed-off-by: Peter Engelbert <[email protected]>
pmengelbert
force-pushed
the
pmengelbert/remove_magic_strings/4
branch
from
19cb65b to
44760a6
Compare
cpuguy83
approved these changes
Jan 23, 2024
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.
Replaces PR #57 , see #57 for all comments which have been addressed by now.
There was a snafu with the Azure/dalec repo going public, so I had to re-fork.