cmd/rofl/build: Validate service volume source#494
Conversation
✅ Deploy Preview for oasisprotocol-cli canceled.
|
80618f6 to
0ed5533
Compare
ac2eb42 to
8c4acf0
Compare
| } | ||
|
|
||
| var ok bool | ||
| for _, v := range proj.Volumes { |
There was a problem hiding this comment.
Would you need to check proj.Volumes too for storage and rofl-appd.sock? Or maybe I didn't get what this check does.
There was a problem hiding this comment.
This checks if the volume is defined in the top-level volumes config.
|
|
||
| // tdxBuildContainer builds a TDX-based container ROFL app. | ||
| func tdxBuildContainer( | ||
| func tdxBuildContainer( //nolint:gocyclo |
There was a problem hiding this comment.
Maybe split into multiple validation functions?
There was a problem hiding this comment.
Splitting validation would also be useful for adding a validation command. For example, for the ROFL dashboard backend, I think it would be useful having something like oasis rofl validate (or oasis rofl build validate), which just does the (quick) validations without actually building the ROFL App. That way, the API will be able to reject invalid manifests quickly before starting the longer-running job to build them.
If you agree to do this, let me know so I open an issue for it :)
There was a problem hiding this comment.
Sounds good :) I'll move the validation into its own function and the rest can be done in a separate PR later.
|
Also there should be a way to force build anyway. |
8c4acf0 to
5790331
Compare
| return true | ||
| } | ||
|
|
||
| if strings.HasPrefix(src, "/storage") { |
There was a problem hiding this comment.
/storage2/mydir should also fail.
| if strings.HasPrefix(src, "/storage") { | |
| if strings.HasPrefix(src, "/storage/") { |
| image = digestBits[0] | ||
| tag = digestBits[1] | ||
| if err := validateComposeFile(artifacts[artifactContainerCompose], manifest); err != nil { | ||
| if common.IsForce() { |
There was a problem hiding this comment.
Use common.CheckForceErr() for standardized force flag check and output.
e41e778 to
0bacfbb
Compare
matevz
left a comment
There was a problem hiding this comment.
LGTM.
Let's open another PR for rofl.yaml and compose.yaml validation.
Closes #462.