add custom CMD (new style entrypoint/cmd) to metatron validation #160
Conversation
entrypoint and command are now verified as arrays by metatron. Backwards and forwards compatibility is being done by the metatron service itself.
Codecov Report
@@ Coverage Diff @@
## master #160 +/- ##
==========================================
- Coverage 33.3% 33.26% -0.04%
==========================================
Files 62 62
Lines 7491 7500 +9
==========================================
Hits 2495 2495
- Misses 4695 4704 +9
Partials 301 301
|
executor/runner/runner.go
Outdated
| c.TitusInfo.MetatronCreds.MetadataSig, | ||
| c.TaskID, | ||
| titusMetadata) | ||
| *c.TitusInfo.MetatronCreds.AppMetadata, |
There was a problem hiding this comment.
Are these all required fields? If not, can you do c.TitusInfo.GetMetatronCreds().GetAppMetadata(), so it wont nil pointer dereference?
There was a problem hiding this comment.
that's a good idea, the old code wasn't checking it (there is a nil check in the code that calls this) so this should be safe as-is. But our future selves will probably appreciate it is a bit safer.
| @@ -170,19 +171,14 @@ func createPassportDir(taskID string) error { | |||
| return os.MkdirAll(getMetatronOutputPath(taskID), os.FileMode(0700)) | |||
| } | |||
|
|
|||
| // RemovePassports removes a task's Metatron credential directory on the host | |||
| func RemovePassports(taskID string) error { | |||
There was a problem hiding this comment.
I notice we removed the call to this function back in May (885ba4b#diff-957e82dddf339c98741109a28268e372). Does that mean we are leaving these creds laying around the host? I realize this is unrelated to this specific PR.
There was a problem hiding this comment.
Yeah. We are. It'd be nice to move to a model where the agent never has to write / read metatron creds from disk, and instead the metatron agent wrote a tarball to stdout.
There was a problem hiding this comment.
Yes, I think you may be right.
There was a problem hiding this comment.
I agree with wanting to avoid writing those creds to disk, but in the meantime seems like we shouldn't be accumulating them on agent (especially since we just saw nfbasic restriction regress). Since it's not really part of this PR, I can file something to at least clean up the creds but to ideally find a way to just pass vs persist them.
Pull Request Test Coverage Report for Build 1760
💛 - Coveralls |
Description of the Change
entrypointandcommandare now verified as arrays by metatron. Backwards and forwards compatibility is being done by the metatron service itself.