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

CfnAssociation outputs invalid CloudFormation #3092

Closed
1 of 5 tasks
kuhnboy opened this issue Jun 26, 2019 · 14 comments · Fixed by #10316
Closed
1 of 5 tasks

CfnAssociation outputs invalid CloudFormation #3092

kuhnboy opened this issue Jun 26, 2019 · 14 comments · Fixed by #10316
Assignees
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager bug This issue is a bug. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 package/cfn Related to the CFN layer (L1)

Comments

@kuhnboy
Copy link

kuhnboy commented Jun 26, 2019

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

  • I'm submitting a ...

    • 🪲 bug report
    • 🚀 feature request
    • 📚 construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    When creating a CfnAssociation parameter:

Parameters = new Dictionary<string, Amazon.CDK.AWS.SSM.CfnAssociation.ParameterValuesProperty>
{ 
  {
    "Operation", new Amazon.CDK.AWS.SSM.CfnAssociation.ParameterValuesProperty
    { 
      ParameterValues = new[] { "Scan" } 
    }
  }
}

The output appears to be correct, but cloud formation reports the following error: "Value of {Parameters} must be a map where each value is a list of {String}"

Looking at the documentation, it appears from the sample yaml it should be:

Parameters:
      Directory: ["myWorkSpace"]

but what is emitted from the code above is:

Parameters:
  Operation:
    ParameterValues:
      - Scan
  • What is the expected behavior (or behavior of feature suggested)?

Would expect the association to be created with the correct parameters and values.

  • What is the motivation / use case for changing the behavior or adding this feature?

Would like to have CfnAssociation working.

  • Please tell us about your environment:

    • CDK CLI Version: 0.28.0
    • Module Version: 0.28.0
    • OS: Windows 10
    • Language: CSharp
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)

@kuhnboy kuhnboy added the needs-triage This issue or PR still needs to be triaged. label Jun 26, 2019
@NGL321 NGL321 added bug This issue is a bug. package/cfn Related to the CFN layer (L1) needs-reproduction This issue needs reproduction. @aws-cdk/aws-ssm Related to AWS Systems Manager language/dotnet Related to .NET bindings and removed needs-triage This issue or PR still needs to be triaged. labels Jun 27, 2019
@eladb eladb assigned eladb and unassigned eladb Aug 12, 2019
@skinny85
Copy link
Contributor

skinny85 commented Sep 5, 2019

@kuhnboy according to the CloudFormation documentation, the first YAML you posted is actually correct... can you tell us what is the error you get from CloudFormation when trying to deploy this?

Thanks,
Adam

@skinny85 skinny85 added needs-response and removed bug This issue is a bug. needs-reproduction This issue needs reproduction. labels Sep 5, 2019
@NGL321 NGL321 added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed status/needs-response labels Sep 16, 2019
@SomayaB SomayaB added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Oct 17, 2019
@tgmedia-nz
Copy link

tgmedia-nz commented Oct 19, 2019

I've got the same problem, this is my synth template:

SSMAssociationApplyPatchBaseline:
    Type: AWS::SSM::Association
    Properties:
      Name: AWS-RunPatchBaseline
      AssociationName: Apply-Patch-Baseline
      Parameters:
        Operations:
          ParameterValues:
            - Install
      Targets:
        - Key: tag:Patching
          Values:
            - enabled

CF error is: "Value of {Parameters} must be a map where each value is a list of {String}"

Using python, I build it (bit retarted tbh, why not just accept dict / array lol):

params = _ssm.CfnAssociation.ParameterValuesProperty(parameter_values = ['Install'])

ssm.CfnAssociation(...
parameters = {"Operations": params})

If I deploy a manually adjusted cfn like this:

      Parameters:
        Operation:
        - Install

CF is deploying it all fine - maybe there is a problem with the AWS documentation?

@SomayaB SomayaB removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Oct 21, 2019
@tgmedia-nz
Copy link

Btw I've submitted feedback to AWS about the documentation being either confusing or wrong :)

@tgmedia-nz
Copy link

tgmedia-nz commented Nov 1, 2019

I've received confirmation from AWS that their documentation is wrong and they will update it:

Thanks for sending along the feedback about the association parametervalues documentation. We’ll get this updated

Best wishes,
-Jim
Jim Brotherton

Amazon Web Services
Senior Technical Writer
AWS Systems Manager

Could you guys fix the CDK part ("Parameters" : {Key : Value, ...}) - cheers

@SomayaB SomayaB added the docs/generated Related to the generated API Reference documentation label Nov 11, 2019
@SomayaB SomayaB assigned rix0rrr and unassigned eladb Nov 11, 2019
@SomayaB SomayaB added feature-request A feature should be added or improved. and removed language/dotnet Related to .NET bindings labels Nov 11, 2019
@rix0rrr rix0rrr added the good first issue Related to contributions. See CONTRIBUTING.md label Nov 27, 2019
@bo67192
Copy link

bo67192 commented Dec 19, 2019

I'm hitting this today with this version

"@aws-cdk/aws-ssm": "^1.19.0",

I have this snippet of CDK code

    new ssm.CfnAssociation(this, 'config-gateway', {
      associationName: 'HC-TSP-Config-Gateway',
      name: 'AWS-RunPowerShellScript',
      scheduleExpression: 'rate(30 minutes)',
      targets: [{key: 'SSM_Association', values: [this.gtw_ssm_assoc]}],
      parameters: {
        commands: {parameterValues: ['return c:\\docutap\\util\\tsp\\Run-GatewayConfig.ps1']}
      }
    })

And I get this cloudformation yaml

  configgateway:
    Type: AWS::SSM::Association
    Properties:
      Name: AWS-RunPowerShellScript
      AssociationName: HC-TSP-Config-Gateway
      Parameters:
        commands:
          ParameterValues:
            - return c:\docutap\util\tsp\Run-GatewayConfig.ps1
      ScheduleExpression: rate(30 minutes)
      Targets:
        - Key: SSM_Association
          Values:
            - PHI-tspgtw-prod
    Metadata:
      aws:cdk:path: tsplus-prod/config-gateway

And the same error about the contents of the parameter field as above.

I'm going to try flipping to an IResolvable produced by this class: https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.Lazy.html

@bo67192
Copy link

bo67192 commented Dec 19, 2019

It looks like there was a workaround in this issue

#4057

I get what looks like valid cloudformation with

    const config_gateway_assoc = new ssm.CfnAssociation(this, 'config-gateway', {
      associationName: 'HC-TSP-Config-Gateway',
      name: 'AWS-RunPowerShellScript',
      scheduleExpression: 'rate(30 minutes)',
      targets: [{key: 'tag:SSM_Association', values: [this.gtw_ssm_assoc]}],
      // parameters: {
      //   commands: cdk.Lazy.anyValue({'return c:\\docutap\\util\\tsp\\Run-GatewayConfig.ps1'})
      // }
    })
    config_gateway_assoc.addPropertyOverride('Parameters.commands', ['return c:\\docutap\\util\\tsp\\Run-GatewayConfig.ps1']);

EDIT: Fixing target syntax, and making Association parameters a list

@kuhnboy
Copy link
Author

kuhnboy commented Jan 10, 2020

@skinny85 I noted above in my original post what the error I was getting from cloud formation:

"Value of {Parameters} must be a map where each value is a list of {String}"

Interesting this hasn't been fixed yet but I as well do not set Parameters and have to do a property override:

`
//Parameters = new Dictionary<string, CfnAssociation.ParameterValuesProperty>
//{
// {
// "Operation", new CfnAssociation.ParameterValuesProperty
// {

			//			ParameterValues = new[] { "Scan" }
			//		}
			//	}
			//},
			ScheduleExpression = "rate(1 day)"
		});

		assoc.AddPropertyOverride("Parameters", new Dictionary<string, string[]>
		{
			{
				"Operation", new[] { "Scan" }
			}
		});

`

@rix0rrr rix0rrr removed the docs/generated Related to the generated API Reference documentation label Jan 23, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 23, 2020

The CloudFormation schema is wrong. Please report this issue to CloudFormation

@rix0rrr rix0rrr added bug This issue is a bug. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 and removed feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md labels Jan 23, 2020
@kuhnboy
Copy link
Author

kuhnboy commented Jan 23, 2020

The CloudFormation schema is wrong. Please report this issue to CloudFormation

What does that mean? It seems that the way cdk is outputting is not correct, cloud formation expects it in a different format. When we output custom code it works. So how is it an issue with cloud formation?

@bo67192
Copy link

bo67192 commented Jan 23, 2020

@rix0rrr I'm happy to open another bug report on the cloudformation side, but can you share what lead you to that conclusion?

@chefren
Copy link
Contributor

chefren commented May 21, 2020

This is still happening.

Contacted AWS support and confirmed they have an internal issue for the documentation, since it states the same as CDK produces. They raised a ticket with the Cloudformation team. Also filed it in their Roadmap: aws-cloudformation/cloudformation-coverage-roadmap#474

The root cause seems to be that specs are missing the AWS::SSM::Association.ParameterValues definition

Related to CDK, something in the build is not quite correct as it appears to be hitting the us-east-1 spec URL, according to this code

update-spec \
    "CloudFormation Resource Specification" \
    "https://d1uauaxba7bl26.cloudfront.net/latest/gzip/CloudFormationResourceSpecification.json" \
    spec-source/000_CloudFormationResourceSpecification.json \
    true

But that spec does not have the definition, so at some point, CDK is hitting another spec (eg: us-east-2) where that exists, therefore producing templates that are not possible to deploy

    "AWS::SSM::Association.ParameterValues": {
      "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ssm-association-parametervalues.html",
      "Properties": {
        "ParameterValues": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ssm-association-parametervalues.html#cfn-ssm-association-parametervalues-parametervalues",
          "DuplicatesAllowed": false,
          "PrimitiveItemType": "String",
          "Required": true,
          "Type": "List",
          "UpdateType": "Mutable"
        }
      }
    },

For a workaround similar to what @bo67192 posted above, see also: #4057 (comment)

@tlinkin
Copy link

tlinkin commented Jul 7, 2020

Issue still active in 1.50

@skinny85
Copy link
Contributor

Confirming. I'm working on a fix.

@mergify mergify bot closed this as completed in #10316 Sep 16, 2020
mergify bot pushed a commit that referenced this issue Sep 16, 2020
There is a problem with the `Parameters` property of the `AWS::SSM::Association` resource type.
Server-side, its type is `Map<String, List<String>>`.
Unfortunately, that type is impossible to represent in the current CloudFormation specification that our code generation is based on.

Add a patch that changes the `Parameters` property of `AWS::SSM::Association` to:

```json
    "AWS::SSM::Association": {
      "Properties": {
        "Parameters": {
          "Type": "Map",
          "ItemType": "List",
          "PrimitiveItemItemType": "String",
          "UpdateType": "Mutable",
          "Required": false
        }
      }
    }
```

That is not a legal CFN spec type.
But we add a new property type, `MapOfListsOfPrimitives`,
to the `cfnspec` module, that represents it.
We then modify our L1 code generation in `cfn2ts` to use the new type,
allowing `CfnAssociationProps.parameters` to have type `{ [key: string]: string[] }`.

Fixes #3092

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@udondan
Copy link
Contributor

udondan commented Sep 16, 2020

Thanks @skinny85! Excited for the next release to try this and remove my overrides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager bug This issue is a bug. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 package/cfn Related to the CFN layer (L1)
Projects
None yet
Development

Successfully merging a pull request may close this issue.