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

fix: Use LimaUser method instead of host user name #712

Merged
merged 4 commits into from
Dec 2, 2023

Conversation

w-haibara
Copy link
Contributor

@w-haibara w-haibara commented Nov 30, 2023

Issue #, if available: #399

Description of changes:
In nerdctl_config_applier.go, editing .bashrc uses a file path based on the host's username. However, if the host's username contains . or @, lima uses lima as the username instead of the host's username.
(ref: https://github.com/lima-vm/lima/blob/a8c703bf8b66d213d00542ef68271cd7b73612ef/pkg/osutil/user.go#L114-L119)
Therefore, if the host's user name contains . or @, editing .bashrc fails.
This PR fixes this so that you can get the actual username from LimaWrapper.LimaUser().

Testing done: yes

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Wataru Haibara <hwhaibarawataru@gmail.com>
@w-haibara w-haibara changed the title fix: use LimaUser method instead of host user name fix: Use LimaUser method instead of host user name Nov 30, 2023
Copy link
Contributor

@vsiravar vsiravar left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this PR. Could you help add a test in nerdctl_config_applier_test.go when local user is not a valid linux username.

Signed-off-by: Wataru Haibara <hwhaibarawataru@gmail.com>
Signed-off-by: Wataru Haibara <hwhaibarawataru@gmail.com>
… not a valid linux username

Signed-off-by: Wataru Haibara <hwhaibarawataru@gmail.com>
@w-haibara
Copy link
Contributor Author

@vsiravar Thanks for review!
I added a test in nerdctl_config_applier_test.go when local user is not a valid linux username.

I have also made additional changes. I would appreciate it if you would review again.

@w-haibara w-haibara requested a review from vsiravar December 1, 2023 17:06
Copy link
Contributor

@vsiravar vsiravar left a comment

Choose a reason for hiding this comment

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

Thanks, the changes look good.

@vsiravar vsiravar merged commit 7c02e08 into runfinch:main Dec 2, 2023
13 checks passed
@w-haibara
Copy link
Contributor Author

Thank you for merge it!

This was referenced Dec 4, 2023
KevinLiAWS pushed a commit that referenced this pull request Dec 7, 2023
🤖 I have created a release *beep* *boop*
---


## [1.0.1](v1.0.0...v1.0.1)
(2023-12-07)


### Bug Fixes

* Clean up all previous finch version installation registries in
postinstall and uninstall
([#688](#688))
([9afc0b9](9afc0b9))
* Fix to be able to run finch build with --ssh option
([#696](#696))
([4d1e6cf](4d1e6cf))
* Fix to delete ~/.finch when uninstalling finch
([#703](#703))
([8d7389f](8d7389f))
* remove virtual machine image when running make clean
([98c8ee4](98c8ee4))
* resolve shellcheck warnings
([#684](#684))
([d9f695a](d9f695a))
* Use LimaUser method instead of host user name
([#712](#712))
([7c02e08](7c02e08))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
KevinLiAWS added a commit that referenced this pull request Dec 11, 2023
🤖 I have created a release *beep* *boop*
---


## [1.0.1](v1.0.0...v1.0.1)
(2023-12-11)


### Bug Fixes

* Change the default behavoir for deleting .finch folder to false when
uninstall ([#732](#732))
([e818743](e818743))
* Clean up all previous finch version installation registries in
postinstall and uninstall
([#688](#688))
([9afc0b9](9afc0b9))
* Fix to be able to run finch build with --ssh option
([#696](#696))
([4d1e6cf](4d1e6cf))
* Fix to delete ~/.finch when uninstalling finch
([#703](#703))
([8d7389f](8d7389f))
* remove virtual machine image when running make clean
([98c8ee4](98c8ee4))
* resolve shellcheck warnings
([#684](#684))
([d9f695a](d9f695a))
* Use LimaUser method instead of host user name
([#712](#712))
([7c02e08](7c02e08))

### Reverts

* "fix: resolve shellcheck warnings"
([#725](#725))
([8ea255a](8ea255a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: Kevin Li <cnkevin@amazon.com>
Co-authored-by: Kevin Li <cnkevin@amazon.com>
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.

2 participants