Skip to content

confusing behavior att init --contract #1694

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

Open
migmartri opened this issue Jan 3, 2025 · 10 comments · May be fixed by #1974
Open

confusing behavior att init --contract #1694

migmartri opened this issue Jan 3, 2025 · 10 comments · May be fixed by #1974
Assignees

Comments

@migmartri
Copy link
Member

when you run for example chainloop att init --workflow my-wf --contract my-contract ... what happens is

1 - a new workflow called my-wf is created (if it doesn't exist) associated with the contract my-contract
2 - a new run is performed

This means that if you after the first run, you provide another contract name chainloop att init --workflow my-wf --contract contract-2, the first contract (my-contract will be used, in other words, the provided contract-2 will be ignored, which was reported as unexpected behavior

@migmartri
Copy link
Member Author

from top of my head we have two options once you run att init after the workflow was created

1 - if the contract you provide with the flag --contract is different than the one from the existing workflow fail the attestation telling the user to change it or
2 - if you provide --contract, make sure to swap it if needed.

I am leaning towards 2, wdyt @jiparis @javirln

@migmartri migmartri assigned migmartri and unassigned jiparis and javirln Jan 3, 2025
@javirln
Copy link
Member

javirln commented Jan 3, 2025

I would say option 2 is more user friendly as long as we always inform the end user about that swap!

Copy link
Member

jiparis commented Jan 3, 2025

  1. is too much a side effect in my opinion. Could we just warn the user that --contract will be ignored if the workflow already exists? Another option is to change the option to something more meaningful for this specific case, like --initial-contract or similar

@migmartri
Copy link
Member Author

@jiparis did we end up implementing this? I remember we talked about it

In any case I've tried it like this as part of the quickstart and didn't work

 chainloop att init \
  --workflow mywf \
  --project myproject \
  --contract https://raw.githubusercontent.com/chainloop-dev/chainloop/refs/heads/main/docs/examples/quickstart/quickstart-contract.yaml --replace

Copy link
Member

jiparis commented Mar 27, 2025

It was implemented here #1902. What's the error?

@migmartri
Copy link
Member Author

It was implemented here #1902. What's the error?

not an error, but instead that I can't see it updating the contract.

@migmartri
Copy link
Member Author

@jiparis it would be nice if you could give it a check to see if we can close this

Copy link
Member

jiparis commented Apr 11, 2025

Sure, att init never updates the contract, that's the confusing part. I'll take a look

@jiparis jiparis assigned jiparis and unassigned migmartri Apr 11, 2025
@migmartri
Copy link
Member Author

Sure, att init never updates the contract, that's the confusing part. I'll take a look

oh, so we didn't add it in the end I guess?

Copy link
Member

jiparis commented Apr 15, 2025

Please check #1974, this should fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants