Skip to content

Conversation

@Ansible-man
Copy link
Contributor

Description

This PR allows users to pass the --interactive flag to zarf tools registry login. This helps prevent credentials from being exposed to bash history and syslog. Fully backwards compatible with the existing flags the --interactive flag will only prompt the user for information they did not provide at command runtime.
...

Related Issue

#4003

Checklist before merging

@Ansible-man Ansible-man requested review from a team as code owners July 20, 2025 03:14
@netlify
Copy link

netlify bot commented Jul 20, 2025

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 55e6560
🔍 Latest deploy log https://app.netlify.com/projects/zarf-docs/deploys/68db376f7d89e8000857ae54

@Ansible-man
Copy link
Contributor Author

Ansible-man commented Jul 20, 2025

I will figure out how to update the docs tomorrow

@brandtkeller
Copy link
Member

I will figure out how to update the docs tomorrow

make docs-and-schema should update the docs accordingly.

@codecov
Copy link

codecov bot commented Jul 21, 2025

Codecov Report

❌ Patch coverage is 13.04348% with 80 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/cmd/crane.go 13.04% 79 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
src/cmd/crane.go 23.62% <13.04%> (-6.21%) ⬇️

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

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

DCO, lint, and Docs and I believe this is in a good place. Functionality performs as expected for me locally.

@cade-thomas
Copy link

cade-thomas commented Jul 21, 2025

I can get this done tonight hopefully - ansible-mans other account

@cade-thomas
Copy link

@brandtkeller Please let me know if there is anything else to be done here and I can get to it this week - ansible-man

@brandtkeller
Copy link
Member

Please let me know if there is anything else to be done here and I can get to it this week - @Ansible-man

DCO needs signing - typically git commit -s resolves this issue. I need to check on our signed commits requirement.

Otherwise the linting job has a few minor items that need to be adjusted otherwise this looks good to me.

Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

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

linting feedback

src/cmd/crane.go Outdated
}
cmd.Flags().Set("password", pass)
}

Copy link
Member

Choose a reason for hiding this comment

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

linting - remove this blank line

@Ansible-man Ansible-man force-pushed the interactive-registry-login branch from edfe951 to 638cea3 Compare September 6, 2025 01:10
@brandtkeller
Copy link
Member

Hey @Ansible-man - looking at git history and surmising what might have gone wrong.

I think we have a couple options - the most clear is going to be reseting:

git checkout interactive-registry-login
git reset --hard d65005dd1bbd09d758f66d7d6a6a15872a7cb0f6
git push --force-with-lease

then re-resolving the linter changes. I'd push that - verify no conflicts - and then update the branch with changes from main. We can do the update once this is back to a clean slate.

@Ansible-man Ansible-man force-pushed the interactive-registry-login branch from 638cea3 to d65005d Compare September 30, 2025 01:42
Signed-off-by: Cade Thomas <[email protected]>
Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

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

Given the issue description and conversation - I believe this fulfills the requirements. Thank you for the contribution! Will tag @zarf-dev/maintainers for a second review.

@AustinAbro321 AustinAbro321 added this pull request to the merge queue Sep 30, 2025
Merged via the queue into zarf-dev:main with commit d9d4c77 Sep 30, 2025
39 of 40 checks passed
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 this pull request may close these issues.

4 participants