feat(attestation): add warning when outdated contract is used #2146
Conversation
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
| } | ||
|
|
||
| // Shows warning if newer contract revision exists | ||
| func (action *AttestationInit) WarnIfOutdatedContract( |
There was a problem hiding this comment.
it might be easier if we extend the init API response to include the latest version if it doesn't do it already
There was a problem hiding this comment.
Ok, in fact I think I am wrong, there is an easier way that does not neither update the API endpoint nor doing an additional request.
The workflow response here should contain the number of the latest revision of the contract available for that workflow
and in that action, you also have the "desired" contract revision provided by the user (note that if you do not provide it, you'll get the default value 0 which means latest, so it's optional)
chainloop/app/cli/cmd/attestation_init.go
Line 90 in 729d68d
So basically the code shoudl just check if the value is provided and smaller that the latest one that comes from the workflow API
What do you think?
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
You are right, I've updated the code. |
|
|
||
| // Shows warning if newer contract revision exists | ||
| func (action *AttestationInit) warnIfOutdatedContract(latestRevision, providedRevision int32) error { | ||
| if action.dryRun || action.useRemoteState || providedRevision == 0 { |
There was a problem hiding this comment.
why if remote state is set we should ignore it?
There was a problem hiding this comment.
We actually shouldn't ignore it, since it does not really matter whether the attestation is stored or where it is stored.
| } | ||
| workflow := workflowsResp.GetResult() | ||
|
|
||
| if err := action.warnIfOutdatedContract(workflow.ContractRevisionLatest, int32(opts.ContractRevision)); err != nil { |
There was a problem hiding this comment.
it feels like this method adds an unnecessary level of indirection. For example it can never return an error.
There was a problem hiding this comment.
Moved remaining logic to Run function.
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Co-authored-by: Miguel Martinez Trivino <migmartri@gmail.com> Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
2 participants
Adds warning when outdated contract revisions are used during attestation:
Closes #1910