Skip to content
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

✨ Implement publish command #146

Merged
merged 2 commits into from
Aug 27, 2024
Merged

✨ Implement publish command #146

merged 2 commits into from
Aug 27, 2024

Conversation

janiskemper
Copy link
Member

What this PR does / why we need it:
csmctl publish will generate the release similar to create command but additionally it will push the generated release to the oci repository.

For this, we implemented the generic interface for assetsclients that we have in the CSO as well and that is fulfilled both by Github and OCI clients.

Which issue(s) this PR fixes:
Fixes #142

TODOs:

  • squash commits
  • include documentation
  • add unit tests

`csmctl publish` will generate the release similar to create command but
additionally it will push the generated release to the oci repository.

For this, we implemented the generic interface for assetsclients that we
have in the CSO as well and that is fulfilled both by Github and OCI
clients.

Signed-off-by: janiskemper <[email protected]>
@cluster-stack-bot cluster-stack-bot bot added size/XXL Denotes a PR that changes 2000+ lines, ignoring generated files. area/code Changes made in the code directory labels Jul 26, 2024
@chess-knight
Copy link
Member

I tried pushing into the Harbor instance, but I was not successful. I set OCI_ env variables, but could not generate a proper access token, because I got a 401 error. Do you know how to do that?
I am also wondering if we should allow username/password auth, maybe it will work better.

@janiskemper
Copy link
Member Author

but could not generate a proper access token, because I got a 401 error.

is this a problem of the registry or of this tool? Sounds to me like registry. Am I correct?

In general I guess it would be doable to implement also a username password flow

@chess-knight
Copy link
Member

but could not generate a proper access token, because I got a 401 error.

is this a problem of the registry or of this tool? Sounds to me like registry. Am I correct?

Yes, most likely this is a problem with the registry or I don't know how to generate a proper registry token. Maybe the problem is with the fact that the SCS registry uses OIDC and that's why I am getting 401 error - see https://github.com/goharbor/harbor/wiki/Harbor-FAQs#api.

In general I guess it would be doable to implement also a username password flow

I tried to change the code with the username/password auth and it works for me.
./csctl publish tests/cluster-stacks/docker/ferrol -m hash resulted into https://registry.scs.community/harbor/projects/39/repositories/docker/artifacts-tab/artifacts/sha256:15f696ec357cd1ad24c7a9bd3d0b2382c16f68d81118a1d3cf5bcf4f5e5e23b4?publicAndNotLogged=yes

@janiskemper
Copy link
Member Author

cool! feel free to commit into this PR or make another PR on this branch. Or do you say this is ready to merge? then we can also merge it. If you want to review, then feel free!

@jschoone
Copy link
Contributor

jschoone commented Aug 9, 2024

Hi @janiskemper and @chess-knight,
I checked out the latest version here with basic auth options and set the env vars like this and skipped the API_TOKEN:

export OCI_REGISTRY=registry.scs.community
export OCI_REPOSITORY=csctl-oci
export OCI_USERNAME=<some-robot-account>
export OCI_PASSWORD=<some-password>

Then I use the csctl publish same as I use csctl create but it errors:

csctl publish ~/workspace/scs/cluster-stacks/providers/openstack/scs  -m hash
path "/home/janjan/workspace/scs/cluster-stacks/providers/openstack/scs": cluster stack hash: "smo7kvy13qhoujfs7le0oot0denufoqn8u1w7nfwii"
.tmp/openstack-scs-1-28-v0-sha-smo7kvy/cluster-addon/Chart.yaml updating version from v1 to v0-sha.smo7kvy
clusterclass chart path: .tmp/openstack-scs-1-28-v0-sha-smo7kvy/cluster-class/Chart.yaml.tmp/openstack-scs-1-28-v0-sha-smo7kvy/cluster-class/Chart.yaml updating version from v1 to v0-sha.smo7kvy
path now: ".tmp/openstack-scs-1-28-v0-sha-smo7kvy/cluster-class"
path now: ".tmp/openstack-scs-1-28-v0-sha-smo7kvy/cluster-addon"
Error: failed to generate release: failed to create new oci client: failed to create OCI client to remote repository csctl-oci: invalid reference: missing registry or repository

What did I do wrong?

@janiskemper
Copy link
Member Author

try the repository registry.scs.community/csctl-oci

@chess-knight
Copy link
Member

export OCI_REPOSITORY=csctl-oci

If I remember correctly, something like this export OCI_REPOSITORY=registry.scs.community/csctl-oci should help.

@jschoone
Copy link
Contributor

Thanks @chess-knight and @janiskemper, adding the registry did the trick. But I also needed to append the provider name, so it was OCI_REPOSITORY=registry.scs.community/csctl-oci/openstack.
Then it worked.

The only thing what is a little annoying is that the command doesn't detect if there's already the same release in the registry and uploads them every time. Usually I run the csctl create many times to get the generated hash out of it to create the ClusterStack yamls automatically. With csctl publish it always pushes it again with a new digest.
Would it make sense to check on the locally generated hash and if it's already there it shouldn't be pushed? At least when using hash mode that should be unique.

@janiskemper
Copy link
Member Author

Hey @jschoone, I checked it and it is true. We should probably check beforehand whether it is there and only if it is not, then we upload it again!

Copy link
Contributor

@jschoone jschoone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jschoone, I checked it and it is true. We should probably check beforehand whether it is there and only if it is not, then we upload it again!

imo even without the check this can be merged for now

@janiskemper janiskemper merged commit a743d0c into main Aug 27, 2024
5 checks passed
@janiskemper janiskemper deleted the publish branch August 27, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code Changes made in the code directory size/XXL Denotes a PR that changes 2000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command to publish assets to OCI registry
3 participants