Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion otdfctl/cmd/policy/resourceMappingGroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ func policyListResourceMappingGroups(cmd *cobra.Command, args []string) {

limit := c.Flags.GetRequiredInt32("limit")
offset := c.Flags.GetRequiredInt32("offset")
nsID := c.Flags.GetOptionalID("namespace-id")
nsFqn := c.Flags.GetOptionalString("namespace-fqn")

resp, err := h.ListResourceMappingGroups(cmd.Context(), limit, offset)
resp, err := h.ListResourceMappingGroups(cmd.Context(), nsID, nsFqn, limit, offset)
if err != nil {
cli.ExitWithError("Failed to list resource mapping groups", err)
}
Expand Down Expand Up @@ -180,6 +182,16 @@ func initResourceMappingGroupsCommands() {
listDoc := man.Docs.GetCommand("policy/resource-mapping-groups/list",
man.WithRun(policyListResourceMappingGroups),
)
listDoc.Flags().String(
listDoc.GetDocFlag("namespace-id").Name,
listDoc.GetDocFlag("namespace-id").Default,
listDoc.GetDocFlag("namespace-id").Description,
)
listDoc.Flags().String(
listDoc.GetDocFlag("namespace-fqn").Name,
listDoc.GetDocFlag("namespace-fqn").Default,
listDoc.GetDocFlag("namespace-fqn").Description,
)
injectListPaginationFlags(listDoc)

updateDoc := man.Docs.GetCommand("policy/resource-mapping-groups/update",
Expand Down
52 changes: 49 additions & 3 deletions otdfctl/cmd/policy/resourceMappings.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ func createResourceMapping(cmd *cobra.Command, args []string) {

attrID := c.Flags.GetRequiredID("attribute-value-id")
grpID := c.Flags.GetOptionalID("group-id")
nsID := c.Flags.GetOptionalID("namespace-id")
nsFqn := c.Flags.GetOptionalString("namespace-fqn")
terms = c.Flags.GetStringSlice("terms", terms, cli.FlagsStringSliceOptions{
Min: 1,
})
metadataLabels = c.Flags.GetStringSlice("label", metadataLabels, cli.FlagsStringSliceOptions{Min: 0})

resourceMapping, err := h.CreateResourceMapping(attrID, terms, grpID, getMetadataMutable(metadataLabels))
resourceMapping, err := h.CreateResourceMapping(attrID, terms, grpID, nsID, nsFqn, getMetadataMutable(metadataLabels))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Pass the command's context (cmd.Context()) to the updated CreateResourceMapping handler method to support proper context propagation.

Suggested change
resourceMapping, err := h.CreateResourceMapping(attrID, terms, grpID, nsID, nsFqn, getMetadataMutable(metadataLabels))
resourceMapping, err := h.CreateResourceMapping(cmd.Context(), attrID, terms, grpID, nsID, nsFqn, getMetadataMutable(metadataLabels))

if err != nil {
cli.ExitWithError("Failed to create resource mapping", err)
}
Expand All @@ -40,6 +42,8 @@ func createResourceMapping(cmd *cobra.Command, args []string) {
{"Terms", strings.Join(resourceMapping.GetTerms(), ", ")},
{"Group Id", resourceMapping.GetGroup().GetId()},
{"Group Name", resourceMapping.GetGroup().GetName()},
{"Namespace Id", resourceMapping.GetNamespace().GetId()},
{"Namespace", resourceMapping.GetNamespace().GetFqn()},
}
if mdRows := getMetadataRows(resourceMapping.GetMetadata()); mdRows != nil {
rows = append(rows, mdRows...)
Expand All @@ -66,6 +70,8 @@ func getResourceMapping(cmd *cobra.Command, args []string) {
{"Terms", strings.Join(resourceMapping.GetTerms(), ", ")},
{"Group Id", resourceMapping.GetGroup().GetId()},
{"Group Name", resourceMapping.GetGroup().GetName()},
{"Namespace Id", resourceMapping.GetNamespace().GetId()},
{"Namespace", resourceMapping.GetNamespace().GetFqn()},
}
if mdRows := getMetadataRows(resourceMapping.GetMetadata()); mdRows != nil {
rows = append(rows, mdRows...)
Expand All @@ -81,8 +87,10 @@ func listResourceMappings(cmd *cobra.Command, args []string) {

limit := c.Flags.GetRequiredInt32("limit")
offset := c.Flags.GetRequiredInt32("offset")
nsID := c.Flags.GetOptionalID("namespace-id")
nsFqn := c.Flags.GetOptionalString("namespace-fqn")

resp, err := h.ListResourceMappings(cmd.Context(), limit, offset)
resp, err := h.ListResourceMappings(cmd.Context(), nsID, nsFqn, limit, offset)
if err != nil {
cli.ExitWithError("Failed to list resource mappings", err)
}
Expand All @@ -94,6 +102,7 @@ func listResourceMappings(cmd *cobra.Command, args []string) {
table.NewFlexColumn("terms", "Terms", cli.FlexColumnWidthFour),
table.NewFlexColumn("group_id", "Group Id", cli.FlexColumnWidthFive),
table.NewFlexColumn("group_name", "Group Name", cli.FlexColumnWidthTwo),
table.NewFlexColumn("namespace", "Namespace", cli.FlexColumnWidthTwo),
table.NewFlexColumn("labels", "Labels", cli.FlexColumnWidthOne),
table.NewFlexColumn("created_at", "Created At", cli.FlexColumnWidthOne),
table.NewFlexColumn("updated_at", "Updated At", cli.FlexColumnWidthOne),
Expand All @@ -107,6 +116,7 @@ func listResourceMappings(cmd *cobra.Command, args []string) {
"attr_value": resourceMapping.GetAttributeValue().GetValue(),
"group_id": resourceMapping.GetGroup().GetId(),
"group_name": resourceMapping.GetGroup().GetName(),
"namespace": resourceMapping.GetNamespace().GetFqn(),
"terms": strings.Join(resourceMapping.GetTerms(), ", "),
"labels": metadata["Labels"],
"created_at": metadata["Created At"],
Expand All @@ -126,10 +136,12 @@ func updateResourceMapping(cmd *cobra.Command, args []string) {
id := c.Flags.GetRequiredID("id")
attrValueID := c.Flags.GetOptionalID("attribute-value-id")
grpID := c.Flags.GetOptionalID("group-id")
nsID := c.Flags.GetOptionalID("namespace-id")
nsFqn := c.Flags.GetOptionalString("namespace-fqn")
terms = c.Flags.GetStringSlice("terms", terms, cli.FlagsStringSliceOptions{})
metadataLabels = c.Flags.GetStringSlice("label", metadataLabels, cli.FlagsStringSliceOptions{Min: 0})

resourceMapping, err := h.UpdateResourceMapping(id, attrValueID, grpID, terms, getMetadataMutable(metadataLabels), getMetadataUpdateBehavior())
resourceMapping, err := h.UpdateResourceMapping(id, attrValueID, grpID, nsID, nsFqn, terms, getMetadataMutable(metadataLabels), getMetadataUpdateBehavior())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Pass the command's context (cmd.Context()) to the updated UpdateResourceMapping handler method to support proper context propagation.

Suggested change
resourceMapping, err := h.UpdateResourceMapping(id, attrValueID, grpID, nsID, nsFqn, terms, getMetadataMutable(metadataLabels), getMetadataUpdateBehavior())
resourceMapping, err := h.UpdateResourceMapping(cmd.Context(), id, attrValueID, grpID, nsID, nsFqn, terms, getMetadataMutable(metadataLabels), getMetadataUpdateBehavior())

if err != nil {
cli.ExitWithError(fmt.Sprintf("Failed to update resource mapping (%s)", id), err)
}
Expand All @@ -140,6 +152,8 @@ func updateResourceMapping(cmd *cobra.Command, args []string) {
{"Terms", strings.Join(resourceMapping.GetTerms(), ", ")},
{"Group Id", resourceMapping.GetGroup().GetId()},
{"Group Name", resourceMapping.GetGroup().GetName()},
{"Namespace Id", resourceMapping.GetNamespace().GetId()},
{"Namespace", resourceMapping.GetNamespace().GetFqn()},
}
if mdRows := getMetadataRows(resourceMapping.GetMetadata()); mdRows != nil {
rows = append(rows, mdRows...)
Expand Down Expand Up @@ -174,6 +188,8 @@ func deleteResourceMapping(cmd *cobra.Command, args []string) {
{"Terms", strings.Join(resourceMapping.GetTerms(), ", ")},
{"Group Id", resourceMapping.GetGroup().GetId()},
{"Group Name", resourceMapping.GetGroup().GetName()},
{"Namespace Id", resourceMapping.GetNamespace().GetId()},
{"Namespace", resourceMapping.GetNamespace().GetFqn()},
}
t := cli.NewTabular(rows...)
common.HandleSuccess(cmd, resourceMapping.GetId(), t, resourceMapping)
Expand All @@ -199,6 +215,16 @@ func initResourceMappingsCommands() {
createDoc.GetDocFlag("group-id").Default,
createDoc.GetDocFlag("group-id").Description,
)
createDoc.Flags().String(
createDoc.GetDocFlag("namespace-id").Name,
createDoc.GetDocFlag("namespace-id").Default,
createDoc.GetDocFlag("namespace-id").Description,
)
createDoc.Flags().String(
createDoc.GetDocFlag("namespace-fqn").Name,
createDoc.GetDocFlag("namespace-fqn").Default,
createDoc.GetDocFlag("namespace-fqn").Description,
)
injectLabelFlags(&createDoc.Command, false)

getDoc := man.Docs.GetCommand("policy/resource-mappings/get",
Expand All @@ -213,6 +239,16 @@ func initResourceMappingsCommands() {
listDoc := man.Docs.GetCommand("policy/resource-mappings/list",
man.WithRun(listResourceMappings),
)
listDoc.Flags().String(
listDoc.GetDocFlag("namespace-id").Name,
listDoc.GetDocFlag("namespace-id").Default,
listDoc.GetDocFlag("namespace-id").Description,
)
listDoc.Flags().String(
listDoc.GetDocFlag("namespace-fqn").Name,
listDoc.GetDocFlag("namespace-fqn").Default,
listDoc.GetDocFlag("namespace-fqn").Description,
)
injectListPaginationFlags(listDoc)

updateDoc := man.Docs.GetCommand("policy/resource-mappings/update",
Expand All @@ -239,6 +275,16 @@ func initResourceMappingsCommands() {
updateDoc.GetDocFlag("group-id").Default,
updateDoc.GetDocFlag("group-id").Description,
)
updateDoc.Flags().String(
updateDoc.GetDocFlag("namespace-id").Name,
updateDoc.GetDocFlag("namespace-id").Default,
updateDoc.GetDocFlag("namespace-id").Description,
)
updateDoc.Flags().String(
updateDoc.GetDocFlag("namespace-fqn").Name,
updateDoc.GetDocFlag("namespace-fqn").Default,
updateDoc.GetDocFlag("namespace-fqn").Description,
)
injectLabelFlags(&updateDoc.Command, true)

deleteDoc := man.Docs.GetCommand("policy/resource-mappings/delete",
Expand Down
6 changes: 6 additions & 0 deletions otdfctl/docs/man/policy/resource-mapping-groups/list.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ command:
aliases:
- l
flags:
- name: namespace-id
description: Filter the list to resource mapping groups owned by this namespace ID.
default: ''
- name: namespace-fqn
description: Filter the list to resource mapping groups owned by this namespace FQN.
default: ''
- name: limit
shorthand: l
description: Limit retrieved count
Expand Down
6 changes: 6 additions & 0 deletions otdfctl/docs/man/policy/resource-mappings/create.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ command:
- name: group-id
description: The ID of the resource mapping group to assign this mapping to
default: ''
- name: namespace-id
description: Optional ID of the namespace that owns this resource mapping. If a group is provided, it must match the group's namespace.
default: ''
- name: namespace-fqn
description: Optional FQN of the namespace that owns this resource mapping (alternative to namespace-id).
default: ''
- name: label
description: "Optional metadata 'labels' in the format: key=value"
shorthand: l
Expand Down
6 changes: 6 additions & 0 deletions otdfctl/docs/man/policy/resource-mappings/list.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ command:
aliases:
- l
flags:
- name: namespace-id
description: Filter the list to resource mappings owned by this namespace ID.
default: ''
- name: namespace-fqn
description: Filter the list to resource mappings owned by this namespace FQN.
default: ''
- name: limit
shorthand: l
description: Limit retrieved count
Expand Down
6 changes: 6 additions & 0 deletions otdfctl/docs/man/policy/resource-mappings/update.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ command:
- name: group-id
description: The ID of the resource mapping group to assign this mapping to
default: ''
- name: namespace-id
description: Optional ID of the namespace that owns this resource mapping. If the mapping belongs to a group, it must match the group's namespace.
default: ''
- name: namespace-fqn
description: Optional FQN of the namespace that owns this resource mapping (alternative to namespace-id).
default: ''
- name: label
description: "Optional metadata 'labels' in the format: key=value"
shorthand: l
Expand Down
4 changes: 3 additions & 1 deletion otdfctl/pkg/handlers/resourceMappingGroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ func (h *Handler) GetResourceMappingGroup(ctx context.Context, id string) (*poli
return res.GetResourceMappingGroup(), nil
}

func (h *Handler) ListResourceMappingGroups(ctx context.Context, limit, offset int32) (*resourcemapping.ListResourceMappingGroupsResponse, error) {
func (h *Handler) ListResourceMappingGroups(ctx context.Context, namespaceID, namespaceFqn string, limit, offset int32) (*resourcemapping.ListResourceMappingGroupsResponse, error) {
return h.sdk.ResourceMapping.ListResourceMappingGroups(ctx, &resourcemapping.ListResourceMappingGroupsRequest{
NamespaceId: namespaceID,
NamespaceFqn: namespaceFqn,
Pagination: &policy.PageRequest{
Limit: limit,
Offset: offset,
Expand Down
12 changes: 9 additions & 3 deletions otdfctl/pkg/handlers/resourceMappings.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import (
)

// Creates and returns the created resource mapping
func (h *Handler) CreateResourceMapping(attributeID string, terms []string, grpID string, metadata *common.MetadataMutable) (*policy.ResourceMapping, error) {
func (h *Handler) CreateResourceMapping(attributeID string, terms []string, grpID, namespaceID, namespaceFqn string, metadata *common.MetadataMutable) (*policy.ResourceMapping, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In Go, it is a best practice to propagate context.Context to downstream handlers and SDK calls rather than using context.Background(). This enables proper cancellation, timeouts, and tracing.

Please update the signature of CreateResourceMapping to accept ctx context.Context as the first parameter, and use it in the CreateResourceMapping SDK call on the next line.

Suggested change
func (h *Handler) CreateResourceMapping(attributeID string, terms []string, grpID, namespaceID, namespaceFqn string, metadata *common.MetadataMutable) (*policy.ResourceMapping, error) {
func (h *Handler) CreateResourceMapping(ctx context.Context, attributeID string, terms []string, grpID, namespaceID, namespaceFqn string, metadata *common.MetadataMutable) (*policy.ResourceMapping, error) {

res, err := h.sdk.ResourceMapping.CreateResourceMapping(context.Background(), &resourcemapping.CreateResourceMappingRequest{
AttributeValueId: attributeID,
GroupId: grpID,
NamespaceId: namespaceID,
NamespaceFqn: namespaceFqn,
Terms: terms,
Metadata: metadata,
})
Expand All @@ -34,8 +36,10 @@ func (h *Handler) GetResourceMapping(id string) (*policy.ResourceMapping, error)
return res.GetResourceMapping(), nil
}

func (h *Handler) ListResourceMappings(ctx context.Context, limit, offset int32) (*resourcemapping.ListResourceMappingsResponse, error) {
func (h *Handler) ListResourceMappings(ctx context.Context, namespaceID, namespaceFqn string, limit, offset int32) (*resourcemapping.ListResourceMappingsResponse, error) {
return h.sdk.ResourceMapping.ListResourceMappings(ctx, &resourcemapping.ListResourceMappingsRequest{
NamespaceId: namespaceID,
NamespaceFqn: namespaceFqn,
Pagination: &policy.PageRequest{
Limit: limit,
Offset: offset,
Expand All @@ -45,12 +49,14 @@ func (h *Handler) ListResourceMappings(ctx context.Context, limit, offset int32)

// TODO: verify updation behavior
// Updates and returns the updated resource mapping
func (h *Handler) UpdateResourceMapping(id string, attrValueID string, grpID string, terms []string, metadata *common.MetadataMutable, behavior common.MetadataUpdateEnum) (*policy.ResourceMapping, error) {
func (h *Handler) UpdateResourceMapping(id, attrValueID, grpID, namespaceID, namespaceFqn string, terms []string, metadata *common.MetadataMutable, behavior common.MetadataUpdateEnum) (*policy.ResourceMapping, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In Go, it is a best practice to propagate context.Context to downstream handlers and SDK calls rather than using context.Background(). This enables proper cancellation, timeouts, and tracing.

Please update the signature of UpdateResourceMapping to accept ctx context.Context as the first parameter, and use it in the UpdateResourceMapping SDK call on the next line.

Suggested change
func (h *Handler) UpdateResourceMapping(id, attrValueID, grpID, namespaceID, namespaceFqn string, terms []string, metadata *common.MetadataMutable, behavior common.MetadataUpdateEnum) (*policy.ResourceMapping, error) {
func (h *Handler) UpdateResourceMapping(ctx context.Context, id, attrValueID, grpID, namespaceID, namespaceFqn string, terms []string, metadata *common.MetadataMutable, behavior common.MetadataUpdateEnum) (*policy.ResourceMapping, error) {

_, err := h.sdk.ResourceMapping.UpdateResourceMapping(context.Background(), &resourcemapping.UpdateResourceMappingRequest{
Id: id,
AttributeValueId: attrValueID,
Terms: terms,
GroupId: grpID,
NamespaceId: namespaceID,
NamespaceFqn: namespaceFqn,
Metadata: metadata,
MetadataUpdateBehavior: behavior,
})
Expand Down
Loading