Skip to content

Adding crane ls command#1835

Merged
Racer159 merged 10 commits intozarf-dev:mainfrom
radiusmethod:feature/crane-ls-command
Jun 26, 2023
Merged

Adding crane ls command#1835
Racer159 merged 10 commits intozarf-dev:mainfrom
radiusmethod:feature/crane-ls-command

Conversation

@dgershman
Copy link
Copy Markdown
Contributor

@dgershman dgershman commented Jun 21, 2023

Description

This adds support for crane ls. This feature is very useful when attempting to do some automation of updating images. The only other way to do this is to run zarf connect registry and then request the tags endpoint of the registry.

Related Issue

N/A

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@netlify
Copy link
Copy Markdown

netlify bot commented Jun 21, 2023 •
edited
Loading

✅ Deploy Preview for zarf-docs ready!

Name Link
🔨 Latest commit 4fac284
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6499da6b1cf8e000089c034f
😎 Deploy Preview https://deploy-preview-1835--zarf-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dgershman dgershman changed the title Feature/crane ls command Adding crane ls command Jun 21, 2023
YrrepNoj
YrrepNoj suggested changes Jun 21, 2023
View reviewed changes
Comment thread src/cmd/tools/crane.go Outdated
Comment thread src/cmd/tools/crane.go Outdated
YrrepNoj
YrrepNoj suggested changes Jun 22, 2023
View reviewed changes
Comment thread src/cmd/tools/crane.go Outdated
Comment thread src/cmd/tools/crane.go Outdated
@dgershman dgershman force-pushed the feature/crane-ls-command branch from ecfdc1b to d2a85c7 Compare June 22, 2023 22:13
Racer159
Racer159 suggested changes Jun 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think the logic overall looks good - added some feedback on internal vs external registry support. Also you'll need to run make docs-and-schema to update the CLI docs. Also as a heads up / note this may hit some small merge conflicts with #1848 as well (edit: the conflicts were really minor so I merged it in)

Comment thread src/cmd/tools/crane.go Outdated
Comment thread src/cmd/tools/crane.go Outdated
Comment thread src/cmd/tools/crane.go Outdated
Comment thread src/cmd/tools/crane.go Outdated
Racer159
Racer159 previously approved these changes Jun 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

lgtm

@Racer159 Racer159 self-requested a review June 26, 2023 18:00
@dgershman dgershman force-pushed the feature/crane-ls-command branch from f7c3f0e to 068f846 Compare June 26, 2023 18:00
@dgershman dgershman requested a review from Madeline-UX as a code owner June 26, 2023 18:00
Racer159
Racer159 suggested changes Jun 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

make docs-and-schema looks like it still needs to be ran

Racer159
Racer159 approved these changes Jun 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm (ah GitHub race conditions...)

@dgershman
Copy link
Copy Markdown
Contributor Author

dgershman commented Jun 26, 2023

I am seeing an issue with make docs-and-schema where its updating all the docs with an extra line break at the end of each markdown file.

@Racer159
Copy link
Copy Markdown
Contributor

Racer159 commented Jun 26, 2023

I am seeing an issue with make docs-and-schema where its updating all the docs with an extra line break at the end of each markdown file.

Hmm that might be because of your OS... What were you devving on?

@dgershman
Copy link
Copy Markdown
Contributor Author

dgershman commented Jun 26, 2023 •
edited
Loading

macOS Ventura 13.4

@Racer159
Copy link
Copy Markdown
Contributor

Racer159 commented Jun 26, 2023

macOS Ventura 13.4

Hmm I mostly run flavors of Linux - might be something with the sed command we run under docs/gen-cli-docs.sh

@Racer159
Copy link
Copy Markdown
Contributor

Racer159 commented Jun 26, 2023

Looks like if you remove one newline from the doc you added that would make it pass.

@dgershman
Copy link
Copy Markdown
Contributor Author

dgershman commented Jun 26, 2023

macOS Ventura 13.4

Hmm I mostly run flavors of Linux - might be something with the sed command we run under docs/gen-cli-docs.sh

Yah. Fixed on my end with an alias to gsed. https://superuser.com/questions/307165/newlines-in-sed-on-mac-os-x

Racer159
Racer159 suggested changes Jun 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

image

Looks like catalog and registry also changed (since ls is a new subcommand of registry).

@Racer159
Copy link
Copy Markdown
Contributor

Racer159 commented Jun 26, 2023

image

Looks like catalog and registry also changed (since ls is a new subcommand of registry).

(might still need catalog to be updated but we'll see... GH doesn't do a good job of showing updates when they happen)

Racer159
Racer159 approved these changes Jun 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

lgtm! 🎉

@Racer159 Racer159 requested a review from YrrepNoj June 26, 2023 18:45
@Racer159 Racer159 enabled auto-merge (squash) June 26, 2023 18:48
@dgershman
Copy link
Copy Markdown
Contributor Author

dgershman commented Jun 26, 2023 •
edited
Loading

PS, I plan to work in crane push and crane pull (as a separate PR) so they also don't require establishing a tunnel if they are referencing an internal image. Is this cool?

@Racer159
Copy link
Copy Markdown
Contributor

Racer159 commented Jun 26, 2023

Yep! That works following the pattern established here. Will definitely make the crane commands more useful.

@Racer159 Racer159 merged commit 09522c4 into zarf-dev:main Jun 26, 2023
@dgershman dgershman mentioned this pull request Jun 26, 2023
Merged
5 tasks
Racer159 added a commit that referenced this pull request Jun 28, 2023
@dgershman @Racer159
Allow for internal crane pulls and pushes without a tunnel + explicit…
dd54f24 
… auth (#1851)

Allow for internal crane pulls and pushes without a tunnel + explicit
credentials

## Description

This gives the same ability as #1835 to `zarf tools crane push` and
`zarf tools crane pull`. Essentially, you shouldn't have to create a
tunnel and auth to the registry to pull or push images to it.

## Related Issue

#1835

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeff-mccoy jeff-mccoy Awaiting requested review from jeff-mccoy

@Noxsios Noxsios Awaiting requested review from Noxsios

@Madeline-UX Madeline-UX Awaiting requested review from Madeline-UX

2 more reviewers

@Racer159 Racer159 Racer159 approved these changes

@YrrepNoj YrrepNoj YrrepNoj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dgershman @Racer159 @YrrepNoj