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

Behaviour Change. Why? #203

Closed
simlu opened this issue Mar 5, 2019 · 13 comments
Closed

Behaviour Change. Why? #203

simlu opened this issue Mar 5, 2019 · 13 comments
Labels

Comments

@simlu
Copy link

simlu commented Mar 5, 2019

In NPM release 2.6.13, the behaviour was as following:

(1) before pushing to aws the basepath was checked
(2) if appropriate the basepath change was injected into the cloudformation template

This worked great. However with the latest release 3.1.0 that was released four days ago all is done outside the cloudformation stack after the stack has fully deployed.

Has caused me a bit of headache since there is also no error details thrown and we have very tight IAM permissions. What is the reason for this change?

Cheers, L~

@simlu simlu added the question label Mar 5, 2019
@simlu
Copy link
Author

simlu commented Mar 5, 2019

I guess my question is really:

Which parts of this plugin are done by "injecting into the stack" and which ones are done "as separate requests outside cloudformation"?

@captainsidd
Copy link
Contributor

Hi there! Thanks for asking, since I'm sure other people have the same question.

In version 3, we decided to create/update/delete all resources through the API. Previously, only the basepath mapping was managed through CloudFormation. We moved away from creating anything through the stack for two reasons:

  1. It seemed cleaner to have all resources be created in the same fashion, rather than just having one created elsewhere. Since multiple CloudFormation stacks can't create the same custom domain, we decided to have everything be done through the API.
  2. We ran into issues such as No base path mapping is being set #57 where the CloudFormation wasn't always being applied.

However, we still add the domain name and distribution domain name to the CloudFormation outputs, preserving the functionality requested in #43 implemented in #47.

@captainsidd captainsidd pinned this issue Mar 11, 2019
@simlu
Copy link
Author

simlu commented Mar 11, 2019

@captainsidd Thank you very much for the reply. That all makes sense.

I have two follow up tasks:

  1. Document this in the Readme (could be a link to your reply?)
  2. Throw more debug information with the errors (I had to debug from my local machine to figure out why our deploy pipeline suddenly failed)

Would you like me to create separate tickets for these?

@jwwisgerhof
Copy link

jwwisgerhof commented Mar 13, 2019

For anyone else having this issue - we had to add
apigateway:PATCH /domainnames/*/basepathmappings to the permissions. This is not currently shown as a requirement anywhere in the docs.

What is extremely unhelpful is that the error logging is completely lacking. The error received from any API call to AWS is discarded and a general error is returned. I had to modify JS files (to print the right error) and deploy locally to find out the problem..

@captainsidd
Copy link
Contributor

@simlu @jwwisgerhof Yes - those are good suggestions. We'll be sure to update the README along with adding better error messages when in DEBUG mode in the near future.

@ashley7070
Copy link

ashley7070 commented Mar 20, 2019

Same issue however apigateway:PATCH /domainnames/*/basepathmappings doesn't work for me. Had to give apigateway:PATCH * temporarily for this to work. Anyone know how I can restrict this further?

Error I am getting on the console:

   [ { op: 'replace',
       path: '/basePath',
       value: 'serverless-starter-service' },
     [length]: 1 ] })
 
  Error --------------------------------------------------
 
  Error: Unable to update basepath mapping.

@captainsidd
Copy link
Contributor

Hi @ashley7070 - have you tried apigateway:PATCH /domainnames/*/basepathmapping? From what I can see in the AWS IAM docs here, the PATCH request we make is only defined for the BasePathMapping resource, not BasePathMappings.

@captainsidd
Copy link
Contributor

Everyone, we've opened a PR with an updated README and better error messaging - let us know if there's any permissions we missed. Thanks!

@ashley7070
Copy link

@captainsidd Unfortunately this didn't do the trick, here is an excerp of my policy file

        {
            "Effect": "Allow",
            "Action": [
                "apigateway:POST"
            ],
            "Resource": [
                "arn:aws:apigateway:*::/domainnames/*/basepathmappings"
            ]
        },
        {
            "Effect": "Allow",
            "Action": [
                "apigateway:PATCH"
            ],
            "Resource": [
                "arn:aws:apigateway:*::/domainnames/*/basepathmapping"
            ]
        },

@captainsidd
Copy link
Contributor

@ashley7070 does that policy file work? It seems to match what we put into the README

@ashley7070
Copy link

@captainsidd Unfortunately it does not work

@ashley7070
Copy link

Ok this is what ended up working:

{
            "Effect": "Allow",
            "Action": "apigateway:PATCH",
            "Resource": "arn:aws:apigateway:*::/domainnames/*/basepathmapping*"
}

@captainsidd
Copy link
Contributor

That makes sense. We're going to go ahead and merge the update PR then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants