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

feat(update): add update attributes handler and Cobra CLI cmd #4

Merged
merged 9 commits into from
Jan 17, 2024
80 changes: 76 additions & 4 deletions cmd/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ This application is a tool to generate the needed files
to quickly create a Cobra application.`,
}

var attrValues []string
var (
attrValues []string
groupBy []string
resourceDependencies []string
)

var attributeGetCmd = &cobra.Command{
Use: "get <id>",
Expand All @@ -51,6 +55,7 @@ var attributeGetCmd = &cobra.Command{
fmt.Println(
cli.NewTabular().
Rows([][]string{
{"Id", strconv.Itoa(int(attr.Id))},
{"Name", attr.Name},
{"Rule", attr.Rule},
{"Values", cli.CommaSeparated(attr.Values)},
Expand All @@ -75,16 +80,17 @@ var attributesListCmd = &cobra.Command{
}

t := cli.NewTable()
t.Headers("Namespace", "Name", "Rule", "Values")
t.Headers("Id", "Namespace", "Name", "Rule", "Values")
for _, attr := range attrs {
t.Row(
strconv.Itoa(int(attr.Id)),
attr.Namespace,
attr.Name,
attr.Rule,
cli.CommaSeparated(attr.Values),
)
}
fmt.Print(t.Render())
fmt.Println(t.Render())
},
}

Expand All @@ -99,7 +105,7 @@ var attributesCreateCmd = &cobra.Command{
flagHelper := cli.NewFlagHelper(cmd)
name := flagHelper.GetRequiredString("name")
rule := flagHelper.GetRequiredString("rule")
values := flagHelper.GetRequiredStringSlice("values", attrValues, cli.FlagHelperStringSliceOptions{
values := flagHelper.GetStringSlice("values", attrValues, cli.FlagHelperStringSliceOptions{
Min: 1,
})
namespace := flagHelper.GetRequiredString("namespace")
Expand Down Expand Up @@ -165,6 +171,59 @@ var attributesDeleteCmd = &cobra.Command{
},
}

// Update one attribute
var attributeUpdateCmd = &cobra.Command{
Use: "update",
Short: "Update an attribute",
Run: func(cmd *cobra.Command, args []string) {
close := cli.GrpcConnect(cmd)
defer close()

flagHelper := cli.NewFlagHelper(cmd)

id := flagHelper.GetRequiredInt32("id")
name := flagHelper.GetRequiredString("name")
rule := flagHelper.GetRequiredString("rule")

values := flagHelper.GetStringSlice("values", attrValues, cli.FlagHelperStringSliceOptions{
Min: 1,
})

groupBy := flagHelper.GetStringSlice("group-by", groupBy, cli.FlagHelperStringSliceOptions{
Min: 0,
})

resourceDependencies := flagHelper.GetStringSlice("resource-dependencies", resourceDependencies, cli.FlagHelperStringSliceOptions{
Min: 0,
})

resourceId := flagHelper.GetRequiredInt32("resource-id")
resourceVersion := flagHelper.GetRequiredInt32("resource-version")
resourceName := flagHelper.GetRequiredString("resource-name")
resourceNamespace := flagHelper.GetRequiredString("resource-namespace")
resourceDescription := flagHelper.GetRequiredString("resource-description")

if _, err := handlers.UpdateAttribute(
id,
name,
rule,
values,
groupBy,
resourceId,
resourceVersion,
resourceName,
resourceNamespace,
resourceDescription,
resourceDependencies,
); err != nil {
cli.ExitWithError("Could not update attribute", err)
return
} else {
fmt.Println(cli.SuccessMessage(fmt.Sprintf("Attribute id: %d updated.", id)))
}
},
}

func init() {
rootCmd.AddCommand(attributesCmd)

Expand All @@ -179,5 +238,18 @@ func init() {
attributesCreateCmd.Flags().StringP("namespace", "s", "", "Namespace of the attribute")
attributesCreateCmd.Flags().StringP("description", "d", "", "Description of the attribute")

attributesCmd.AddCommand(attributeUpdateCmd)
attributeUpdateCmd.Flags().Int32P("id", "i", 0, "Id of the attribute")
attributeUpdateCmd.Flags().StringP("name", "n", "", "Name of the attribute")
attributeUpdateCmd.Flags().StringP("rule", "r", "", "Rule of the attribute")
attributeUpdateCmd.Flags().StringSliceVarP(&attrValues, "values", "v", []string{}, "Values of the attribute")
attributeUpdateCmd.Flags().StringSliceVarP(&groupBy, "group-by", "g", []string{}, "GroupBy of the attribute")
attributeUpdateCmd.Flags().StringSliceVarP(&resourceDependencies, "resource-dependencies", "d", []string{}, "ResourceDependencies of the attribute definition descriptor")
attributeUpdateCmd.Flags().Int32P("resource-id", "I", 0, "ResourceId of the attribute definition descriptor")
attributeUpdateCmd.Flags().Int32P("resource-version", "V", 0, "ResourceVersion of the attribute definition descriptor")
attributeUpdateCmd.Flags().StringP("resource-name", "N", "", "ResourceName of the attribute definition descriptor")
attributeUpdateCmd.Flags().StringP("resource-namespace", "S", "", "ResourceNamespace of the attribute definition descriptor")
attributeUpdateCmd.Flags().StringP("resource-description", "D", "", "ResourceDescription of the attribute definition descriptor")

attributesCmd.AddCommand(attributesDeleteCmd)
}
19 changes: 18 additions & 1 deletion pkg/cli/flagValues.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ func (f FlagHelper) GetRequiredString(flag string) string {
return v
}

func (f FlagHelper) GetRequiredStringSlice(flag string, v []string, opts FlagHelperStringSliceOptions) []string {
func (f FlagHelper) GetOptionalString(flag string) string {
return f.cmd.Flag(flag).Value.String()
}

func (f FlagHelper) GetStringSlice(flag string, v []string, opts FlagHelperStringSliceOptions) []string {
if len(v) < opts.Min {
fmt.Println(ErrorMessage(fmt.Sprintf("Flag %s must have at least %d non-empty values", flag, opts.Min), nil))
os.Exit(1)
Expand All @@ -40,3 +44,16 @@ func (f FlagHelper) GetRequiredStringSlice(flag string, v []string, opts FlagHel
}
return v
}

func (f FlagHelper) GetRequiredInt32(flag string) int32 {
v, e := f.cmd.Flags().GetInt32(flag)
if e != nil {
fmt.Println(ErrorMessage("Flag "+flag+" is required", nil))
os.Exit(1)
}
// if v == 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will we ever allow zero's as our int32 values passed by users? For example, I ran into an issue here with resource-version. If that value must be defined by a User in an Attribute - Update flow, the version should currently be truly 0, but 0 is also the Go zero-value for the int32 type, so if we want to require an int32 flag, we'd want to gracefully handle these zero values.

Copy link
Member

Choose a reason for hiding this comment

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

I think if the user has to apply the version that is a problem. It seems like the version should iterate.

Copy link
Member

Choose a reason for hiding this comment

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

// fmt.Println(ErrorMessage("Flag "+flag+" must be greater than 0", nil))
// os.Exit(1)
// }
return v
}
65 changes: 62 additions & 3 deletions pkg/handlers/attribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
)

type Attribute struct {
Id int32
Name string
Rule string
Values []string
Expand All @@ -41,12 +42,13 @@ func GetAttribute(id int) (Attribute, error) {
}

return Attribute{
Id: resp.Definition.Descriptor_.Id,
Name: resp.Definition.Name,
Rule: GetAttributeRuleFromAttributeType(resp.Definition.Rule),
Values: values,
Namespace: resp.Definition.Descriptor_.Namespace,
Description: resp.Definition.Descriptor_.Description,
Fqn: GetAttributeFqn(resp.Definition),
Fqn: GetAttributeFqn(resp.Definition.Descriptor_.Namespace, resp.Definition.Name),
}, nil
}

Expand All @@ -64,6 +66,7 @@ func ListAttributes() ([]Attribute, error) {
values = append(values, v.Value)
}
attrs = append(attrs, Attribute{
Id: attr.Descriptor_.Id,
Name: attr.Name,
Rule: GetAttributeRuleFromAttributeType(attr.Rule),
Values: values,
Expand Down Expand Up @@ -110,6 +113,62 @@ func CreateAttribute(name string, rule string, values []string, namespace string
}, nil
}

func UpdateAttribute(
id int32,
name string,
rule string,
values []string,
groupBy []string,
resourceId int32,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I get greater clarity on which of these are generated at the server/db layer and which are truly required by a User of tructl? Some of these like resourceVersion and fqn seem like they could always be ignored/defaulted "clientside"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is looking at the .proto definitions the best place or is there somewhere it's clearer?

Copy link
Member

Choose a reason for hiding this comment

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

I think so, but this is good feedback for the team and a issue is ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resourceVersion int32,
resourceName string,
resourceNamespace string,
resourceDescription string,
resourceDependencies []string,
) (*attributesv1.UpdateAttributeResponse, error) {
var attrValues []*attributesv1.AttributeDefinitionValue
for _, v := range values {
if v != "" {
attrValues = append(attrValues, &attributesv1.AttributeDefinitionValue{Value: v})
}
}

var attrGroupBy []*attributesv1.AttributeDefinitionValue
for _, v := range groupBy {
if v != "" {
attrGroupBy = append(attrGroupBy, &attributesv1.AttributeDefinitionValue{Value: v})
}
}

var dependencies []*commonv1.ResourceDependency
for _, v := range resourceDependencies {
if v != "" {
dependencies = append(dependencies, &commonv1.ResourceDependency{Namespace: v})
}
}

client := attributesv1.NewAttributesServiceClient(grpc.Conn)
return client.UpdateAttribute(grpc.Context, &attributesv1.UpdateAttributeRequest{
Id: id,
Definition: &attributesv1.AttributeDefinition{
Name: name,
Rule: GetAttributeRuleFromReadableString(rule),
Values: attrValues,
GroupBy: attrGroupBy,
Descriptor_: &commonv1.ResourceDescriptor{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If some of these values are optional, should we be careful of zero values in the grpc calls (i.e. nil instead of an empty string slice) or is that handled elegantly in the service when lengths are zero? Looking at Definition.GroupBy which I know is optional and Definition.Descriptor_.Dependencies which I'm unsure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update, looks like resource dependencies (Definition.Descriptor_.Dependencies) are also optional so we should clarify if we need to pass nil to the rpc or if an empty slice is okay.

Copy link
Member

Choose a reason for hiding this comment

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

Yea I think this is relevant opentdf/platform#33

Type: commonv1.PolicyResourceType_POLICY_RESOURCE_TYPE_ATTRIBUTE_DEFINITION,
Id: resourceId,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will the Definition.Descriptor_.Id always be the same as the top-level Id in the request? It seems like there's a (possibly buggy) behavior in the service-layer where passing a top-level Id that matches a real attribute Id and a different Definition.Descriptor_.Id will return a success response but no actual update/change applied on a subsequent get/list.

Version: resourceVersion,
Name: resourceName,
Namespace: resourceNamespace,
Fqn: GetAttributeFqn(resourceNamespace, resourceName),
Description: resourceDescription,
Dependencies: dependencies,
},
},
})
}

func DeleteAttribute(id int) error {
client := attributesv1.NewAttributesServiceClient(grpc.Conn)

Expand All @@ -120,8 +179,8 @@ func DeleteAttribute(id int) error {
return err
}

func GetAttributeFqn(resp *attributesv1.AttributeDefinition) string {
return fmt.Sprintf("https://%s/attr/%s", resp.Descriptor_.Namespace, resp.Name)
func GetAttributeFqn(namespace string, name string) string {
return fmt.Sprintf("https://%s/attr/%s", namespace, name)
}

func GetAttributeRuleOptions() []string {
Expand Down