-
Notifications
You must be signed in to change notification settings - Fork 16
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
Support configuration from remote resources #91
Merged
phlogistonjohn
merged 16 commits into
samba-in-kubernetes:master
from
phlogistonjohn:jjm-config-uris
Oct 23, 2023
Merged
Support configuration from remote resources #91
phlogistonjohn
merged 16 commits into
samba-in-kubernetes:master
from
phlogistonjohn:jjm-config-uris
Oct 23, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
phlogistonjohn
force-pushed
the
jjm-config-uris
branch
3 times, most recently
from
September 19, 2023 20:10
9757902
to
a0e5c79
Compare
phlogistonjohn
force-pushed
the
jjm-config-uris
branch
from
October 14, 2023 14:09
a0e5c79
to
ba18bbd
Compare
Add the concept of an opener type, something that can fetch either local (path-based) or non-local (uri-based) resources. Will be used later to optionally fetch configuration from a remote store. Signed-off-by: John Mulligan <[email protected]>
Add a URLOpener based on python's urllib, but with non-HTTP(s) protocols disabled. The URLOpener can be extended later with other remote storage protocols. Signed-off-by: John Mulligan <[email protected]>
Add a function to conditionally enable RADOS support for the URLOpener. In the future, this will allow sambacc configuration to be stored as a RADOS object, similar to how NFS-Ganesha allows configuration to be stored in RADOS, but without any direct changes to Samba. Signed-off-by: John Mulligan <[email protected]>
Add support to config.py so that an opener can be used in lieu of directly opening local files by path. Signed-off-by: John Mulligan <[email protected]>
When building the initial sambacc configuration, make sure we support passing URL and optionally enable rados support. To support passing urls/uris no changes to the CLI parsing needs to happen but the env var parsing previously assumed a colon-separated list of paths much like the PATH env var. Keep that as a viable option for backwards compatibility but also allow the env var to contain a JSON-formatted list of strings. Example: ``` [0] $ export SAMBACC_CONFIG='["http://localhost:8080/my/config.json", "/my/config.json"]' [0] $ samba-container --identity demo <...> ``` Signed-off-by: John Mulligan <[email protected]>
Signed-off-by: John Mulligan <[email protected]>
Signed-off-by: John Mulligan <[email protected]>
Signed-off-by: John Mulligan <[email protected]>
Just something I noticed while I was looking over coverage for the new types. Signed-off-by: John Mulligan <[email protected]>
Move the private implementation of the minimal FileOpener opener type to the opener.py module. This will allow other parts of sambacc to reuse that opener. Update the config test to match the changes. Signed-off-by: John Mulligan <[email protected]>
Allow passing of URIs in addition to local paths for the Joiner type by supporting an optional opener argument to the Joiner. Signed-off-by: John Mulligan <[email protected]>
Fix how the Joiner and the test for interactive login was using stdin. When the test was run without pytest capturing the stdio, the test would hang waiting on input. This behavior was incorrect. Add a hook to the Joiner for the unit tests to use so that the stdin of the interactive join command can be manipulated. Signed-off-by: John Mulligan <[email protected]>
This opener property is the top-level point for fetching the opener to be used for this instance of sambacc. Signed-off-by: John Mulligan <[email protected]>
When performing a join or must-join command accept an opener provided by the application context. Signed-off-by: John Mulligan <[email protected]>
Add the optional rados dependency. Signed-off-by: John Mulligan <[email protected]>
Signed-off-by: John Mulligan <[email protected]>
phlogistonjohn
force-pushed
the
jjm-config-uris
branch
from
October 14, 2023 14:17
ba18bbd
to
75d278f
Compare
phlogistonjohn
changed the title
[RFC] Support configuration from remote resources
Support configuration from remote resources
Oct 14, 2023
I finally had some time to retest the rados support manually. Looked OK. Ready for review. |
synarete
approved these changes
Oct 15, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(A note for the future: better split such large PR into multiple small-PRs in order to simplify review).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add initial support for sourcing configuration from remote URLs (URIs), in addition to the existing support for local paths.
Currently supports HTTP and HTTPS urls and optionally "pseudo URLs" for resources stored in Ceph RADOS. The latter look like "rados://pool/ns/obj_key".
If the rados library for python is not present sambacc will skip enabling rados support.
Previously, the configuration paths to sambacc could be passed by
--config
cli options or theSAMBACC_CONFIG
environment variable. In the case of the latter we accepted a colon-separated list of paths in the style ofPATH
. However this doesn't work well for uris so it now optionally accpts a JSON-formatted list of strings in the following style:Using JSON is detected by a square bracket as the first and last(ish) character of the env var (trailing whitespace is allowed)'
TODO
Needs more manual testing for RADOS, etc.Extend opener support to join files