fix(core): stack.urlSuffix is no longer scoped

[rix0rrr]

stack.urlSuffix used to be a scoped Token. By the use of roles defined
in another stack (using a ServicePrincipal which uses a urlSuffix
token), this could lead to unintentional stack references.

Two changes here:

• URL Suffix (seems to) only change when Partition changes. Since
  Partition is unscoped (cross-partition references won't work anyway),
  we might as well make URL Suffix unscoped too.
• ServicePrincipalToken should not have used the stack's urlSuffix,
  but constructed an unscoped URL_SUFFIX itself, since it was
  never intended to potentially create a cross-stack reference. It
  couldn't have, since it doesn't know where it is being defined,
  it just knows where it's being used.

Technically, the second change isn't necessary anymore after we
apply the first, but I made both anyway since the bug is still
resolved even if find out we need roll back the first change
because of a future region build.

Fixes #3970.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[mergify]

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

• PR title type(scope): text
  • type: fix, feat, refactor go into CHANGELOG, chore is hidden
  • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
  • text: use all lower-case, do not end with a period, do not include issue refs
• PR Description
  • Rationale: describe rationale of change and approach taken
  • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
  • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
• Testing
  • Unit test added. Prefer to add a new test rather than modify existing tests
  • CLI or init templates change? Re-run/add CLI integration tests
• Documentation
  • README: update module README to describe new features
  • API docs: public APIs must be documented. Copy from official AWS docs when possible
  • Design: for significant features, follow design process

[nija-at]

Do we still need the methods partition() and urlSuffix() in the ScopedAws class?

https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/core/lib/cfn-pseudo.ts#L58

[nija-at]

By the use of roles defined in another stack (using a ServicePrincipal which uses a urlSuffix
token), this could lead to unintentional stack references.`

You should expand on what you mean here, so we can record this for posterity on when someone is looking over why this change was made. I couldn't gather what's going on from the linked issue.

[rix0rrr]

Do we still need the methods partition() and urlSuffix() in the ScopedAws class?

I don't see a reason to remove them. It's something you might want.

You should expand on what you mean here.

Is that worth a provisional approval or would you like to see my rephrasing before you okay it?

[mergify]

Thank you for contributing! Your pull request is now being automatically merged.

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository