-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate keystone workflows + Enforce full semver on capabilities #13328
Conversation
Quality Gate passedIssues Measures |
6a91682
to
77fa66b
Compare
642bc07
to
07f2d48
Compare
e5b96f4
to
ebca00d
Compare
"missing name", | ||
func() string { | ||
id := "15c631d295ef5e32deb99a10ee6804bc4af1385568f9b3363f6552ac6dbb2cef" | ||
owner := "00000000000000000000000000000000000000aa" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The owner needs to be within the workflow spec (not the "job spec for the workflow"), as the nodes will need to get the owner from the workflow after validating signatures, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too sure, I kept the structure of the test cases the same as trunk. I think we should address this in a separate topic as I want to get validation for workflow specs (not job specs) merged in first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DeividasK It's not part of the workflow spec (at least it wasn't last time I checked). We do pass it from the workflow spec to the RequestMetadata thus making it available to capabilities though
d9bc305
to
4e7cc57
Compare
This PR contains the following changes:
CapabilityInfo
struct now embeds the version as part of the ID itself, rather than having it as a separate field.Prior arts:
Related PR: smartcontractkit/chainlink-common#536