-
Notifications
You must be signed in to change notification settings - Fork 218
Prompt for resources with optional resourceType #4535
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ type LocationFilterPredicate func(loc account.Location) bool | |
type Prompter interface { | ||
PromptSubscription(ctx context.Context, msg string) (subscriptionId string, err error) | ||
PromptLocation(ctx context.Context, subId string, msg string, filter LocationFilterPredicate) (string, error) | ||
PromptResource(ctx context.Context, msg string, resourceType string) (string, error) | ||
PromptResourceGroup(ctx context.Context) (string, error) | ||
} | ||
|
||
|
@@ -111,6 +112,51 @@ func (p *DefaultPrompter) PromptLocation( | |
return loc, nil | ||
} | ||
|
||
// PromptResource uses the console (or external) prompter to allow the user to select a resource | ||
// of optional type `resourceType`. The selected resource Name will be returned. | ||
// If no resources of that type were found, an empty string is returned. | ||
// | ||
// The Name can be used with the ARM or Bicep template function `reference` or in an existing resource template's name | ||
// to get provisioned state data from a resource, or passed to the `resourceId` function to get the full resource ID | ||
// if you know the `resourceType`. | ||
func (p *DefaultPrompter) PromptResource( | ||
ctx context.Context, | ||
msg string, | ||
resourceType string, | ||
) (string, error) { | ||
options := azapi.ListResourcesOptions{ | ||
ResourceType: resourceType, | ||
} | ||
resources, err := p.resourceService.ListResources(ctx, p.env.GetSubscriptionId(), &options) | ||
if err != nil { | ||
return "", fmt.Errorf("listing resources: %w", err) | ||
} | ||
if len(resources) == 0 { | ||
return "", nil | ||
} | ||
Comment on lines
+134
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should return an error. Since the intention of the method is to prompt, the fact of an empty list is a blocker to do the prompting, so it should return an error to the caller and report that there's no resources to prompt for. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, my main thought with separating optional out was to also allow more permutations for other selector types. I've seen some APIs that have things like |
||
|
||
slices.SortFunc(resources, func(a, b *azapi.Resource) int { | ||
return strings.Compare(a.Name, b.Name) | ||
}) | ||
|
||
// TODO: Add `optional` field to allow "None" and return ""? | ||
choices := make([]string, len(resources)) | ||
for idx, resource := range resources { | ||
// TODO: Get location display names from account manager instead? | ||
choices[idx] = fmt.Sprintf("%d. %s (%s)", idx+1, resource.Name, resource.Location) | ||
} | ||
|
||
choice, err := p.console.Select(ctx, input.ConsoleOptions{ | ||
Message: msg, | ||
Options: choices, | ||
}) | ||
heaths marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return "", fmt.Errorf("selecting resource: %w", err) | ||
} | ||
|
||
return resources[choice].Name, nil | ||
} | ||
|
||
func (p *DefaultPrompter) PromptResourceGroup(ctx context.Context) (string, error) { | ||
// Get current resource groups | ||
groups, err := p.resourceService.ListResourceGroup(ctx, p.env.GetSubscriptionId(), nil) | ||
|
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.
As I was implementing #4943, I realized that this listing would list resources in the subscription, in which case returning just the resource
Name
isn't enough to fully qualify the resource ID.Returning resource ID is obviously more powerful, and still portable, but the Bicep referencing logic is rather messy. There isn't first-party functions that support this, so it ends up being a lot of string splitting with known indexing. An example of working code that is quite unappealing can be found here.
As @JeffreyCA kindly noted, this may be simpler when Bicep supports dedicated function parsing for resource IDs.
All this is to say that our hands are a little tied on making a nice little prompt experience here, and I'm honestly not sure which way we want to lean, but I did end up returning resource ID in #4943 because subscription-scope resources were a requirement there.