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

gvk partial inference #19058

Merged
merged 8 commits into from
Oct 24, 2023
Merged
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
4 changes: 0 additions & 4 deletions command/resource/delete/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ func (c *cmd) Run(args []string) int {
return 1
}
} else {
if len(args) < 2 {
c.UI.Error("Incorrect argument format: Must specify two arguments: resource type and resource name")
return 1
}
var err error
gvk, resourceName, err = resource.GetTypeAndResourceName(args)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion command/resource/delete/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestResourceDeleteInvalidArgs(t *testing.T) {
"invalid resource type format": {
args: []string{"a.", "name", "-namespace", "default"},
expectedCode: 1,
expectedErr: errors.New("Incorrect argument format: Must include resource type argument in group.verion.kind format"),
expectedErr: errors.New("Must provide resource type argument with either in group.verion.kind format or its shorthand name"),
},
}

Expand Down
83 changes: 57 additions & 26 deletions command/resource/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,7 @@ func parseJson(js string) (*pbresource.Resource, error) {
}

func ParseResourceFromFile(filePath string) (*pbresource.Resource, error) {
data, err := helpers.LoadDataSourceNoRaw(filePath, nil)
if err != nil {
return nil, fmt.Errorf("Failed to load data: %v", err)
}
var parsedResource *pbresource.Resource
if isHCL([]byte(data)) {
parsedResource, err = resourcehcl.Unmarshal([]byte(data), consul.NewTypeRegistry())
} else {
parsedResource, err = parseJson(data)
}
if err != nil {
return nil, fmt.Errorf("Failed to decode resource from input file: %v", err)
}

return parsedResource, nil
return ParseResourceInput(filePath, nil)
}

// this is an inlined variant of hcl.lexMode()
Expand Down Expand Up @@ -165,23 +151,17 @@ func ParseInputParams(inputArgs []string, flags *flag.FlagSet) error {
}

func GetTypeAndResourceName(args []string) (gvk *GVK, resourceName string, e error) {
if len(args) < 2 {
return nil, "", fmt.Errorf("Must specify two arguments: resource type and resource name")
}
// it has to be resource name after the type
if strings.HasPrefix(args[1], "-") {
return nil, "", fmt.Errorf("Must provide resource name right after type")
}
resourceName = args[1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we missing a len check here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is already sanitized before calling this function:
read
delete

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case I think it's better if we change the signature of this func to not take a slice but 2 arguments.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to re-crafted the function signature to receive GetTypeAndResourceName(resourceType string, resourceName string) then I found I need to do the sanity check

if strings.HasPrefix(args[1], "-") {

at the caller side which shifted more code the caller side. So I moved the arguments sanity check inside this function.


s := strings.Split(args[0], ".")
if len(s) != 3 {
return nil, "", fmt.Errorf("Must include resource type argument in group.verion.kind format")
}

gvk = &GVK{
Group: s[0],
Version: s[1],
Kind: s[2],
}
gvk, e = inferGVKFromResourceType(args[0])

resourceName = args[1]
return
}

Expand Down Expand Up @@ -282,3 +262,54 @@ func (resource *Resource) List(gvk *GVK, q *client.QueryOptions) (*ListResponse,

return out, nil
}

func inferGVKFromResourceType(resourceType string) (*GVK, error) {
s := strings.Split(resourceType, ".")
switch length := len(s); {
// only kind is provided
case length == 1:
kindToGVKMap := BuildKindToGVKMap()
kind := strings.ToLower(s[0])
switch len(kindToGVKMap[kind]) {
// no g.v.k is found
case 0:
return nil, fmt.Errorf("The shorthand name does not map to any existing resource type, please check `consul api-resources`")
// only one is found
case 1:
// infer gvk from resource kind
gvkSplit := strings.Split(kindToGVKMap[kind][0], ".")
return &GVK{
Group: gvkSplit[0],
Version: gvkSplit[1],
Kind: gvkSplit[2],
}, nil
// it alerts error if any conflict is found
default:
return nil, fmt.Errorf("The shorthand name has conflicts %v, please use the full name", kindToGVKMap[s[0]])
}
case length == 3:
return &GVK{
Group: s[0],
Version: s[1],
Kind: s[2],
}, nil
default:
return nil, fmt.Errorf("Must provide resource type argument with either in group.verion.kind format or its shorthand name")
}
}

func BuildKindToGVKMap() map[string][]string {
// this use the local copy of registration to build map
typeRegistry := consul.NewTypeRegistry()
kindToGVKMap := map[string][]string{}
for _, r := range typeRegistry.Types() {
gvkString := fmt.Sprintf("%s.%s.%s", r.Type.Group, r.Type.GroupVersion, r.Type.Kind)
kindKey := strings.ToLower(r.Type.Kind)
if len(kindToGVKMap[kindKey]) == 0 {
kindToGVKMap[kindKey] = []string{gvkString}
} else {
kindToGVKMap[kindKey] = append(kindToGVKMap[kindKey], gvkString)
}
}
return kindToGVKMap
}
2 changes: 1 addition & 1 deletion command/resource/read/read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestResourceReadInvalidArgs(t *testing.T) {
"invalid resource type format": {
args: []string{"a.", "name", "-namespace", "default"},
expectedCode: 1,
expectedErr: errors.New("Incorrect argument format: Must include resource type argument in group.verion.kind format"),
expectedErr: errors.New("Incorrect argument format: Must provide resource type argument with either in group.verion.kind format or its shorthand name"),
},
}

Expand Down