Skip to content

Stat Identity file on sys con add - #27199

Open
inknos wants to merge 1 commit into
podman-container-tools:mainfrom
inknos:issue-26016
Open

Stat Identity file on sys con add#27199
inknos wants to merge 1 commit into
podman-container-tools:mainfrom
inknos:issue-26016

Conversation

@inknos

@inknos inknos commented Sep 30, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes: #26016

Does this PR introduce a user-facing change?

Fail fast for `podman system connection add` when id file does not exist or has bad permissions

@openshift-ci openshift-ci Bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Sep 30, 2025
@inknos
inknos marked this pull request as draft September 30, 2025 15:35
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 30, 2025
@inknos
inknos force-pushed the issue-26016 branch 2 times, most recently from 1500121 to 3125ae2 Compare September 30, 2025 15:55
Luap99
Luap99 reviewed Sep 30, 2025
View reviewed changes
Comment thread cmd/podman/system/connection/add.go Outdated
Comment thread cmd/podman/system/connection/add.go Outdated
Comment thread test/system/272-system-connection.bats Outdated
@packit-as-a-service

packit-as-a-service Bot commented Sep 30, 2025

Copy link
Copy Markdown

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

@inknos

inknos commented Sep 30, 2025

Copy link
Copy Markdown
Contributor Author

thanks @Luap99, didn't know about the comments you addressed. I'll fix it tomorrow morning

Luap99
Luap99 reviewed Oct 2, 2025
View reviewed changes

@Luap99 Luap99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes code wise this is fine, just some minor points about the testing

Comment thread test/system/272-system-connection.bats Outdated
Comment thread test/system/272-system-connection.bats Outdated
@inknos
inknos marked this pull request as ready for review October 3, 2025 14:16
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 3, 2025
@openshift-ci openshift-ci Bot added release-note and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Oct 3, 2025
@TomSweeneyRedHat

TomSweeneyRedHat commented Oct 3, 2025

Copy link
Copy Markdown
Contributor

LGTM
once the unhappy tests are happy

@inknos
Stat Identity file on sys con add
aac08c1 
Fixes: podman-container-tools#26016

Signed-off-by: Nicola Sella <nsella@redhat.com>
@openshift-ci

openshift-ci Bot commented Oct 30, 2025

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: inknos
Once this PR has been reviewed and has the lgtm label, please assign tomsweeneyredhat for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Honny1

Honny1 commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

@inknos, can you please rebase and take a look at the tests?

@Honny1

Honny1 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@inknos Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Luap99 Luap99 Luap99 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

release-note

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug(Windows): using podman system connection add with a ~ should convert it to absolute path

4 participants

@inknos @TomSweeneyRedHat @Honny1 @Luap99