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

"sam deploy" does not execute if a tag has a long value #1845

Closed
AaronChelvan opened this issue Mar 1, 2020 · 21 comments
Closed

"sam deploy" does not execute if a tag has a long value #1845

AaronChelvan opened this issue Mar 1, 2020 · 21 comments
Labels
stage/needs-investigation Requires a deeper investigation

Comments

@AaronChelvan
Copy link

Description:
The sam deploy command hangs if a tag with a long value (approx 30+ characters long) is provided. Tested on SAM CLI v0.43.0.

Steps to reproduce the issue:

  1. Create an empty directory and navigate to it.
  2. Execute sam deploy --stack-name test --tags key=012345678901234567890123456789

Observed result:
The command hangs and does not produce any output.

Expected result:

	Deploying with following values
	===============================
	Stack name                 : test
	Region                     : None
	Confirm changeset          : False
	Deployment s3 bucket       : None
	Capabilities               : null
	Parameter overrides        : {}

Initiating deployment
=====================
Error: Failed to package template: /tmpDir/template.yml. 
 [Errno 2] No such file or directory: '/tmpDir/template.yml'

However, if a shorter tag value is provided (e.g. sam deploy --stack-name test --tags key=01234567890123456789) then the correct output gets printed.

@ShreyaGangishetty ShreyaGangishetty transferred this issue from aws/serverless-application-model Mar 4, 2020
@sriram-mv sriram-mv added the stage/needs-investigation Requires a deeper investigation label Mar 5, 2020
@beck3905
Copy link

beck3905 commented Mar 6, 2020

This appears to be caused by this regex that was added in version 0.42.0: https://github.com/awslabs/aws-sam-cli/blob/develop/samcli/cli/types.py#L207.

I isolated this code and tried it with a short tag and a long tag and it is the re.findall call that is hanging. Not exactly sure what the regex pattern is meant to do as it is a rather long and complex pattern. A little more commenting for that would be helpful.

@dslove
Copy link

dslove commented Mar 28, 2020

In my case there is no any error msg showing up. It took me a day to figure out what the problem is and it was because I was lucky to google and find this page... I am using 0.44.

@iDVB
Copy link

iDVB commented Apr 11, 2020

SAM CLI, version 0.47.0
aws-cli/1.16.208 Python/3.7.7 Darwin/18.6.0 botocore/1.12.198
macOS 10.14.5

I too am hitting this issue as well.
I can provide a bit more testing cases.

Using the AWS CLI directly does work fine

aws cloudformation deploy \
  --template .aws-sam/build/packaged.yaml \
  --stack-name 'my-stack-name' \
  --capabilities CAPABILITY_NAMED_IAM \
  --tags \
      SERVICE=ggg-ggggggggg-ggggggg \
      GIT_BRANCH=master \
      GIT_REPOSITORY=gggggg-ggggggggg-ggg-gggggggggggggg

Using the same call with AWS SAM either does not work... or takes way too long

sam deploy \
 --template-file .aws-sam/build/packaged.yaml \
  --stack-name 'my-stack-name' \
  --capabilities CAPABILITY_NAMED_IAM \
--no-fail-on-empty-changeset \
  --tags \
      SERVICE=ggg-ggggggggg-ggggggg \
      GIT_BRANCH=master \
      GIT_REPOSITORY=gggggg-ggggggggg-ggg-gggggggggggggg

What I've noticed is that there seems to even be an exponential execute time increase when I increase that GIT_REPOSITORY value from 24 (4s) to 28 (49s) characters. So I didn't even want to stick around to test 35 characters.

Unfortunately, I'm in the middle of authoring a course, showcasing AWS SAM, that needs this working. 😞

@beck3905
Copy link

@sriram-mv @jfuss Has anyone looked into this issue yet? I noticed that v0.48 of the cli was just released and this has been an issue since v0.42.0. I have had to freeze on v0.41.0 because of this issue.

Why was this regex added to being with? It is not clear what it is supposed to do or look for, but the pattern is terribly non-performant.

@c2tarun
Copy link
Contributor

c2tarun commented May 5, 2020

Thanks @beck3905 for linking the code where the issue. On adding some extra logging, the regex pattern we are using is (\"(?:\\[A-Za-z0-9\"_:\.\/\+-\@=]|[^\"\\]+)*\"|(?:\\[A-Za-z0-9\"_:\.\/\+-\@=]|[^ \"\\]+)+)=(\"(?:\\[A-Za-z0-9\"_:\.\/\+-\@=]|[^\"\\]+)*\"|(?:\\[A-Za-z0-9\"_:\.\/\+-\@=]|[^ \"\\]+)+) this pattern is used to separate key and value of tag. Now, I haven't deep dived into regex but testing on regex101.com shows that we are running into Regex: Catastrophic Backtracking issue.

To add to that, I don't think this regex is completely bug free. For Eg: AWS EC2 Tagging restriction mention that you can use = in key or value. However, if we enter abc=pqr=xyz with our current regex, it'll split it as abc=pqr and xyz instead of abc and pqr=xyz. Both are correct BTW.

Usually we try to stay consistent with how AWS CLI handles things however different services across AWS have different way to handle tagging. Eg:

  1. AWS S3 accepts a JSON array like here.
  2. AWS EC2 accepts Key=key,Value=value type format here.

Plus we don't want to break backwards compatibility for our current users.

I'll sync with team and try to update here on what approach we want to take to fix this, but given that we are already broken with:

  1. Long values.
  2. = in value.
  3. Multiple = in key=value format.

We can probably consider not supporting = in key or value. Please let me know your thoughts around this approach.

@beck3905
Copy link

beck3905 commented May 5, 2020

@c2tarun Thanks for digging into this. I'm not sure about whether or not to support the = in key or value. My use case is broken due to the long values issue. My CI/CD system tags the stack with the URL of the GitHub commit that triggered the deployment. That URL is much longer than the regex can handle and causes our deployments to hang. I don't believe there are any = in that URL, but = are supported in URL's.

@c2tarun
Copy link
Contributor

c2tarun commented May 5, 2020

@beck3905 Yeah, on giving this more thought there might be people who are base64 encoding something as value. Even though we are parsing it wrong, but breaking on multiple = will downright break them. I'll try to reach out to CFN team internally to see if they have some guideline/stricter-restriction around tagging. Will update here after that.

@c2tarun
Copy link
Contributor

c2tarun commented May 5, 2020

Actually we don't have to worry about approaches taken by other AWS Services as long as we stay consistent with AWS CFN. CFN interestingly supports both approaches taken by S3 and EC2 :)

The solution I think will work is:

  1. Do not support = in key and value if using tagName=tagValue format of adding tags.
  2. We should start supporting Key=string,Value=string format for tagging where user can use = after Key= or Value=.
  3. We should start supporting [{Key=string,Value=string},{Key=string,Value=string}] format and user can use = with no issue.

It is little more work then I thought, but should solve most of the problems. There is still a risk if breaking someone if they are already using multiple = but we will print descriptive error message to suggest to use Key=string,Value=string approach, which I think will help in the long run.

@c2tarun
Copy link
Contributor

c2tarun commented May 5, 2020

Correction in my investigation, in order to use = in key or value we can use double quotes. So we can use "abc"="pqr=xyz" and this works.

@micklove
Copy link

@c2tarun Thanks for digging into this. I'm not sure about whether or not to support the = in key or value. My use case is broken due to the long values issue. My CI/CD system tags the stack with the URL of the GitHub commit that triggered the deployment. That URL is much longer than the regex can handle and causes our deployments to hang. I don't believe there are any = in that URL, but = are supported in URL's.

FYI - I have the same problem, tagging the stack with the git commit url (using github actions), hangs the sam deploy command. (Same tagging on cloudformation deploy works fine.)
nb: The sam deploy command works in the OSX terminal.

@c2tarun
Copy link
Contributor

c2tarun commented May 15, 2020

We just released v0.49.0. Sorry for the delay, you should be able to upgrade now. Please feel free to re-open this ticket if you have any questions.

@c2tarun c2tarun closed this as completed May 15, 2020
@beck3905
Copy link

beck3905 commented Jul 9, 2020

@c2tarun I am still experiencing hangs in my CI/CD environment due to long tags. It works eventually on my OSX terminal, but on my CircleCI environment it hangs and the job times out. Even in my OSX terminal, it takes 6+ minutes from when I start the sam deploy command and it prints the telemetry blurb until I see the following output printed to the terminal: Deploying with following values. My guess is that it takes longer in my CI/CD environment due to more limited memory. My longest tag is 40 characters. I also played around with that tag by removing it altogether, shortening it, etc. and each of those made the command run much faster locally. When I shortened the tag and added a character at a time back to it and reran the sam deploy command, each additional character added over a minute to the lag. It still seems that the regex pattern being used to validate the tags is far too complicated and inefficient. I'd like to reopen this issue.

@c2tarun
Copy link
Contributor

c2tarun commented Jul 9, 2020

@beck3905 Can you please share a sample tag similar to what you are using, 6+ minutes on OSX is still too long and we should address it. I want to test with your tag once.

Thanks
Tarun

@beck3905
Copy link

beck3905 commented Jul 9, 2020

@c2tarun These are similar to the tags entry in my samconfig.toml. In particular, the Name=repository-branch-network-infrastructure tag is the one that I have experimented with for length.

tags = "Component=repository-branch-deployment-role [email protected] Department=department Environment=dev Name=repository-branch-network-infrastructure Project=project-name Purpose=development"

@beck3905
Copy link

beck3905 commented Jul 9, 2020

I went back to version 0.41.0, which was the last version before any of the regex stuff was added and everything is working great and quick with that version.

@beck3905
Copy link

beck3905 commented Aug 6, 2020

@c2tarun Any updates on this? I'm still stuck using v 0.41.0 until this gets addressed.

@c2tarun
Copy link
Contributor

c2tarun commented Aug 7, 2020

Hey @beck3905 , sorry and thanks a lot for your patience. This fell off my radar. I can confirm that I am able to reproduce issue with sam deploy --tags="a=b Name=repository-branch-network-infrastructure"

I am re-opening this issue.

Interestingly sam deploy --tags="Name=repository-branch-network-infrastructure" works for me. So it is a combination of a long tag value and one more tag.

@c2tarun
Copy link
Contributor

c2tarun commented Aug 7, 2020

Hello @beck3905, I was able to merge a fix for this issue yesterday. I don't have exact release date yet, but when we'll do next release you should be able to upgrade. I'll leave this issue open, until that happens.

@c2tarun
Copy link
Contributor

c2tarun commented Aug 14, 2020

Hello @beck3905, we have released fix for this issue in our release 1.1.0. Can you please confirm once if this works for you?

Thanks
Tarun

@sriram-mv
Copy link
Contributor

Closing this issue, please re-open if this still an issue.

@esteban1983cl
Copy link

Hello I have an issue with --tags parameter. In OS X sam deploy works well but not using docker images like python-3.8.8-alpine-3.13

Execute this command:

sam deploy --template-file ./templates/${CI_ENVIRONMENT_NAME}/package.yaml --stack-name AWSConfigEventListener --capabilities CAPABILITY_IAM --no-confirm-changeset --s3-bucket ${BUCKET_NAME} --region $AWS_REGION --tags $TAGS
Usage: sam deploy [OPTIONS]
Try 'sam deploy -h' for help.
Error: Invalid value for '--tags': ('ambiente=staging', 'pais=cl', 'ceco=cgo1007324', 'cuenta=xxxxxxx', 'proyecto="Plataformas', 'TxD"', 'aplicacion="DevOps', 'Tools"', 'nombre=AWSConfigEventListener', 'Name=AWSConfigEventListener', '[email protected]', 'tribe="Arquitectura', 'y', 'Plataforma"', 'epm=opex', 'ApplicationName="Plataformas', 'TxD"', 'BusinessUnit=SHARED') is not in valid format. It must look something like 'KeyName1=string KeyName2=string'

where the $TAGS are:

ambiente=staging pais=cl ceco=cgo1007324 cuenta=xxxxxxxxx proyecto="Plataformas TxD" aplicacion="DevOps Tools" nombre=AWSConfigEventListener Name=AWSConfigEventListener [email protected] tribe="Arquitectura y Plataforma" epm=opex ApplicationName="Plataformas TxD" BusinessUnit=SHARED

Is there a workaround to solve this ?

Thanks in advance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/needs-investigation Requires a deeper investigation
Projects
None yet
Development

No branches or pull requests

8 participants