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

Can't use number parameters in addon templates and pass them to integer fields with !Ref #1565

Closed
Envek opened this issue Oct 23, 2020 · 3 comments · Fixed by #1804
Closed
Labels
type/bug Issues that are bugs.

Comments

@Envek
Copy link

Envek commented Oct 23, 2020

If I specify parameter of type Number in addon template and use it with !Ref in a field that requires integer, copilot deploy will fail:

: json: cannot unmarshal string into Go struct field Template.Resources of type int

It is convenient to use parameters to DRY values in templates and keep them app configuration in sync with infrastructure:

Parameters:
  MinPort:
    Type: Number
    Default: 40000
  MaxPort:
    Type: Number
    Default: 49999

Resources:
  EnvironmentSecurityGroupIngressTCPWebRTC:
    Type: AWS::EC2::SecurityGroupIngress
    Properties:
      Description: Ingress TCP for WebRTC media streams
      GroupId:
        Fn::ImportValue:
          !Sub "${App}-${Env}-EnvironmentSecurityGroup"
      IpProtocol: tcp
      CidrIp: '0.0.0.0/0'
      FromPort: !Ref MinPort # <- Problem is here
      ToPort:   !Ref MaxPort # <- Problem is here

Outputs:
  MediasoupMinPort:
    Value: !Ref MinPort
  MediasoupMaxPort:
    Value: !Ref MaxPort

But if I execute this template with aws cloudformation create-stack it will be created successfully.

Related issues (as copilot use goformation): awslabs/goformation#252 and awslabs/goformation#304.

Full example here

copilot/app/addons/ingress-rules.yml:

Parameters:
  App:
    Type: String
    Description: Your application's name.
  Env:
    Type: String
    Description: The environment name your service, job, or workflow is being deployed to.
  MediasoupMinPort:
    Type: Number
    Default: 40000
    Description: "Start of the port range used to receive WebRTC connections with media streams from peers."
    MinValue: 1024
    MaxValue: 65535
  MediasoupMaxPort:
    Type: Number
    Default: 49999
    Description: "End of the port range used to receive WebRTC connections with media streams from peers."
    MinValue: 1024
    MaxValue: 65535

Resources:
  EnvironmentSecurityGroupIngressTCPWebRTC:
    Type: AWS::EC2::SecurityGroupIngress
    Properties:
      Description: Ingress TCP for WebRTC media streams
      GroupId:
        Fn::ImportValue:
          !Sub "${App}-${Env}-EnvironmentSecurityGroup"
      IpProtocol: tcp
      CidrIp: '0.0.0.0/0'
      FromPort: !Ref MediasoupMinPort
      ToPort:   !Ref MediasoupMaxPort

  EnvironmentSecurityGroupIngressUDPWebRTC:
    Type: AWS::EC2::SecurityGroupIngress
    Properties:
      Description: Ingress UDP for WebRTC media streams
      GroupId:
        Fn::ImportValue:
          !Sub "${App}-${Env}-EnvironmentSecurityGroup"
      IpProtocol: udp
      CidrIp: '0.0.0.0/0'
      FromPort: !Ref MediasoupMinPort
      ToPort:   !Ref MediasoupMaxPort

Outputs:
  MediasoupMinPort:
    Description: "Start of the port range used to receive WebRTC connections with media streams from peers."
    Value: !Ref MediasoupMinPort
  MediasoupMaxPort:
    Description: "End of the port range used to receive WebRTC connections with media streams from peers."
    Value: !Ref MediasoupMaxPort

How to run it via CloudFormation:

app=panel-discussions
env=test
aws cloudformation create-stack --stack-name $app-$env-vpc-subnets --template-body file://copilot/app/addons/ingress-rules.yml --parameters ParameterKey=App,ParameterValue=$app ParameterKey=Env,ParameterValue=$env --tags Key=copilot-application,Value=$app Key=copilot-environment,Value=$env
@efekarakus
Copy link
Contributor

Heya @Envek !

Like you pointed out this is an underlying issue with the goformation library that we use :( we have a backlog item to write our own custom logic to avoid these errors.

Sorry about the inconvenience!

@efekarakus efekarakus added the type/bug Issues that are bugs. label Oct 23, 2020
@efekarakus
Copy link
Contributor

Related #1402

@efekarakus efekarakus added this to Backlog in Sprint 🏃‍♀️ via automation Jan 4, 2021
@efekarakus efekarakus moved this from Backlog to In progress in Sprint 🏃‍♀️ Jan 4, 2021
efekarakus added a commit to efekarakus/copilot-cli that referenced this issue Jan 4, 2021
Fixes aws#1565

Also removes the goformation library in favor of the official YAML
library for Go.
By using the yaml library, we get two additional benefits:
- Gets rid of a parsing bug from the goformation lib aws#1402.
- Reduces our binary size by 9MB.
   Before:
   ```
   ls -lh bin/local
   56M copilot
   ```
   After:
   ```
   ls -lh bin/local
   47M copilot
   ```
@mergify mergify bot closed this as completed in #1804 Jan 6, 2021
Sprint 🏃‍♀️ automation moved this from In progress to Pending release Jan 6, 2021
mergify bot pushed a commit that referenced this issue Jan 6, 2021
)

Fixes #1565

Also removes the goformation library in favor of the official YAML library for Go.
By using the yaml library, we get two additional benefits:
- Gets rid of a parsing bug from the goformation lib #1402.
- Reduces our binary size by 9MB.
   Before:
   ```
   ls -lh bin/local
   56M copilot
   ```
   After:
   ```
   ls -lh bin/local
   47M copilot
   ```


_By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
@efekarakus
Copy link
Contributor

This was released just now in v1.1.0: https://github.com/aws/copilot-cli/releases/tag/v1.1.0!

thrau pushed a commit to localstack/copilot-cli-local that referenced this issue Dec 9, 2022
…s#1804)

Fixes aws#1565

Also removes the goformation library in favor of the official YAML library for Go.
By using the yaml library, we get two additional benefits:
- Gets rid of a parsing bug from the goformation lib aws#1402.
- Reduces our binary size by 9MB.
   Before:
   ```
   ls -lh bin/local
   56M copilot
   ```
   After:
   ```
   ls -lh bin/local
   47M copilot
   ```


_By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Issues that are bugs.
Projects
Sprint 🏃‍♀️
  
Pending release
Development

Successfully merging a pull request may close this issue.

2 participants