search

aws / aws-tools-for-powershell

The AWS Tools for PowerShell lets developers and administrators manage their AWS services from the PowerShell scripting environment.
Apache License 2.0
238 stars 79 forks source link

Get-EC2InstanceAttribute Parameter Issues #57

Closed jedatwork closed 4 years ago

jedatwork commented 4 years ago

Get-EC2InstanceAttribute has 2 parameters, -InstanceId and -Attrubte both of the parameters are marked as Required? False, but both parameters actually are required for the function to execute. image image

Also, it doesn't seem like Attribute actually should be required at this point because all attributes are reported back when a single Attribute is specified: image I checked on this and it looks like it would actually take a change in the underlying Amazon.EC2.AmazonEC2Client DescribeInstanceAttributeAsync method, where InstanceId and Attribute are required, but all attributes are returned.

Expected Behavior Get-EC2InstanceAttribute without providing -InstanceId should prompt for an InstanceId similar to how other required parameter functions work, e.g. in Get-Item path is required and prompts for input when not specified image

Current Behavior Get-EC2InstanceAttribute with no -InstanceId throws an error image

Possible Solution Fixing the prompt for InstanceId should just require updating GetEC2InstanceAttributeCmdlet to include ", Mandatory = true" where the InstanceId parameter is being built.

It would be a nice add if the requirement for Attribute were dropped from the underlying DescribeInstanceAttributeAsync method. This seems like if attribute is a requirement only one attribute should be returned (this looks like how it used to work) and if all attributes are going to be returned then the attribute requirement is no longer serving a purpose.

Steps to Reproduce (for bugs) Import-Module AWSPowerShell.NetCore Set-AWSCredential someprofile Set-DefaultAWSRegion us-east-1 Get-EC2InstanceAttribute Context I was really just looking at the help to see what were valid values for the Attribute argument and noticed that the Required flags do not accurately reflect what the underlying method call requires.

Your Environment

Include as many relevant details about the environment where the bug was discovered.

tommymaynard commented 4 years ago

I worked with Get-EC2InstanceAttribute from the AWS.Tools.EC2, PreRelease module (3.3.590.0) on PowerShell 6.2.3 on Windows.

When the command is invoked without any parameters, it prompts for a value for the Attribute and InstanceId parameters, as though they are both required. Even so, the included help still indicates neither is required. I agree, the help should be updated for the next release.

I also agree that this command should internally filter down to the specifically requested attribute. Otherwise, this parameter doesn't make much sense. I recommend to fix that, or remove the Attribute parameter all together, and always return all the attributes (and therefore, require the user do the filtering [Where-Object]).

Thank you for creating this issue @jedatwork.

matteo-prosperi commented 4 years ago

Hello, Thanks for your feedback.

Mandatory parameters are enforced in AWS.Tools, the new modular version of AWS Tools for PowerShell. See here. AWSPowerShell and AWSPowerShell.NetCore don't prompt for required parameters (with the exception of a few special cmdlets). The documentation (both web cmdlet reference and Get-Help pages) will be updated in the next release for all cmdlets to include which parameters are required. This is a snippet of the new output:

NAME
    Get-EC2InstanceAttribute

SYNOPSIS
    Calls the Amazon Elastic Compute Cloud DescribeInstanceAttribute API operation.

SYNTAX
    Get-EC2InstanceAttribute [-InstanceId] <System.String> [-Attribute] <Amazon.EC2.InstanceAttributeName> [<CommonParameters>]

DESCRIPTION
    Describes the specified attribute of the specified instance. You can specify only one attribute at a time. Valid attribute values are: instanceType | kernel | ramdisk | userData | disableApiTermination | instanceInitiatedShutdownBehavior | rootDeviceName | blockDeviceMapping | productCodes | sourceDestCheck | groupSet | ebsOptimized | sriovNetSupport

PARAMETERS
    -Attribute <Amazon.EC2.InstanceAttributeName>
        The instance attribute.
        Note: The enaSupport attribute is not supported at this time.

        Required?                    true
        Position?                    2
        Default value                None
        Accept pipeline input?       True (ByPropertyName)
        Accept wildcard characters?  false

    -InstanceId <System.String>
        The ID of the instance.

        Required?                    true
        Position?                    1
        Default value                None
        Accept pipeline input?       True (ByValue, ByPropertyName)
        Accept wildcard characters?  false

The behavior of this cmdlet is tied to the DescribeInstanceAttribute API. Changing this behavior would be backward incompatible with existing scripts.

matteo-prosperi commented 4 years ago

The docs (https://docs.aws.amazon.com/powershell/latest/reference/items/Get-EC2InstanceAttribute.html) now report mandatory parameters. Get-Help does as well starting with version 4.0 of the modules. As said above, only the AWS.Tools module actually prompt for mandatory parameters.