admin/smb: add more flexible generic resource types#1223
Merged
Conversation
9b0791b to
c4cd261
Compare
anoopcs9
reviewed
Feb 12, 2026
c4cd261 to
216d152
Compare
Collaborator
Author
|
Well, it looks like I fouled up the changes to the Convert function somehow. |
anoopcs9
reviewed
Feb 12, 2026
b35fdca to
71a62eb
Compare
When running the Show function to get resources from the server add a private field that can be used to customize how the unmarshaling is done. This will be used in a future change to support generic smb resource types. Because this new field is private this does not need to be part of a preview API. Accessor functions that manipulate it in the future will. Signed-off-by: John Mulligan <jmulligan@redhat.com>
The concrete Cluster, Share, etc. types are all nice and useful but with a rapidly evolving smb mgr module are bound to be perpetually out of date. Add a new GenericResource type that is a thin layer over the JSON input/output of the smb mgr module so that bleeding-edge or experimental features can still be manipulated somewhat easily through this go-ceph library. An example workflow might involve creating concrete Cluster and Share instances and filling in the common fields the usual way, then converting them into generic resources to set experimental branch fields on the Shares via the generic Values map before submitting them to the mgr via the Apply method. Methods to convert to and from the more concrete types (Cluster, Share, etc) exist specifically to help enable this workflow. New methods to ShowOpts allow requesting that the library return GenericResourc instances when querying the mgr. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
71a62eb to
67b400b
Compare
ansiwen
reviewed
Feb 18, 2026
ansiwen
left a comment
Collaborator
There was a problem hiding this comment.
I can't really oversee if certain things are safe or not, so I just left some questions. Otherwise LGTM.
| func (t TopIdentityKind) Identity(m map[string]any) ResourceRef { | ||
| return ResourceID{ | ||
| ResourceType: getResourceType(m), | ||
| ID: m[t.IDKey].(string), |
Collaborator
There was a problem hiding this comment.
is it guaranteed by business-logic, that this key exists in the map?
Collaborator
Author
There was a problem hiding this comment.
If it doesn't its not a valid resource. The server always produces a resource with an id field of form and if you try to construct a Generic Resource from scratch and don't set up an id it will be invalid. If that means this function panics I am OK with that because it is a programmer error.
| ) | ||
|
|
||
| func getResourceType(m map[string]any) ResourceType { | ||
| return ResourceType(m[ResourceTypeKey].(string)) |
Collaborator
There was a problem hiding this comment.
Just to make sure: collisions with "resrouce_type" are not possible?
Collaborator
Author
There was a problem hiding this comment.
You can manually mangle "resource_type" but then it won't pass validation.
| // Data is stored in the Values map and metadata used to identify the | ||
| // resource is provided by the IDKind field. | ||
| type GenericResource struct { | ||
| Values map[string]any |
Collaborator
There was a problem hiding this comment.
Why is this "any"? Is this an external dependency?
Collaborator
Author
There was a problem hiding this comment.
We can not predict that the values in the top-level map are strings, ints, lists of strings, lists of maps of strings, etc.
As long as we can round-trip between JSON we really can work with any type here so I used any which is the newer way to express a map of string to any type.
ansiwen
approved these changes
Feb 18, 2026
Collaborator
|
Approved, remove the label to merge. |
Contributor
Merge Queue StatusRule:
This pull request spent 7 seconds in the queue, with no time running CI. Required conditions to merge
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The concrete Cluster, Share, etc. types are all nice and useful but with a rapidly evolving smb mgr module are bound to be perpetually out of date. Add a new GenericResource type that is a thin layer over the JSON input/output of the smb mgr module so that bleeding-edge or experimental features can still be manipulated somewhat easily through this go-ceph library.
An example workflow might involve creating concrete Cluster and Share instances and filling in the common fields the usual way, then converting them into generic resources to set experimental branch fields on the Shares via the generic Values map before submitting them to the mgr via the Apply method.
Methods to convert to and from the more concrete types (Cluster, Share, etc) exist specifically to help enable this workflow. New methods to ShowOpts allow requesting that the library return GenericResource instances when querying the mgr.
Thus the generic resource type can be used as a stop-gap or longer any time the go-ceph admin/smb package doesn't support some resource fields that the mgr module does.
Checklist
//go:build ceph_previewmake api-updateto record new APIsNew or infrequent contributors may want to review the go-ceph Developer's Guide including the section on how we track API Status and the API Stability Plan.
The go-ceph project uses mergify. View the mergify command guide for information on how to interact with mergify. Add a comment with
@Mergifyiorebaseto rebase your PR when github indicates that the PR is out of date with the base branch.