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

If no NFS ACLs provided, assume everyone #548

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

benjamreis
Copy link
Contributor

Signed-off-by: BenjiReis benjamin.reis@vates.fr

Solves: #511

As detailed in the issue, some devices (mostly QNAS devices) do not provide with NFS ACLs in showmount -e return, in this case we need to assume everyone is accepted, therefore access = "*".

drivers/nfs.py Outdated Show resolved Hide resolved
@benjamreis
Copy link
Contributor Author

Can showmount be mocked to fake a result with no access in it?

@MarkSymsCtx
Copy link
Contributor

Can showmount be mocked to fake a result with no access in it?

Of course, if you know what the response looks like, just make the return value from the mock look like that.

@benjamreis
Copy link
Contributor Author

Okay thanks, I'll add a test then :)

@benjamreis benjamreis force-pushed the do-not-crash-if-no-nfs-acl branch 2 times, most recently from 2738f6f to 93379ab Compare February 17, 2021 09:44
@benjamreis
Copy link
Contributor Author

Test added and successfull, this is good to review, and that's one more nfs method covered by the test! 👍

@benjamreis benjamreis changed the title If not NFS ACLs provided, assume everyone If no NFS ACLs provided, assume everyone Feb 25, 2021
Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
@benjamreis
Copy link
Contributor Author

Hi!

Any changes required? Or something is missing? Tests and test coverage should be fine.

Regards!

Copy link
Contributor

@MarkSymsCtx MarkSymsCtx left a comment

Choose a reason for hiding this comment

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

Built and tested.

@MarkSymsCtx MarkSymsCtx merged commit fb42c07 into xapi-project:master Jun 16, 2021
@stormi stormi deleted the do-not-crash-if-no-nfs-acl branch March 8, 2022 16:32
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.

3 participants