fix: stack.partition is never scoped

[rix0rrr]

A fully specified ARN can now be used in a different stack without
leading to a Fn::ImportValue (used to be that we incurrend an
ImportValue because of the Partition, but that is silly because
the variable is known anyway).

BREAKING CHANGE: 'Aws' class returns unscoped Tokens, introduce a
new class 'ScopedAws' which returns scoped Tokens.

Fixes #1755

---

Pull Request Checklist

• Testing
  • Unit test added
  • CLI change?: coordinate update of integration tests with team
  • cdk-init template change?: coordinated update of integration tests with team
• Docs
  • jsdocs: All public APIs documented
  • README: README and/or documentation topic updated
• Title and Description
  • Change type: title prefixed with fix, feat will appear in changelog
  • Title: use lower-case and doesn't end with a period
  • Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
  • Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
• Sensitive Modules (requires 2 PR approvers)
  • IAM Policy Document (in @aws-cdk/aws-iam)
  • EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
  • Grant APIs (only if not based on official documentation with a reference)

---

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

[eladb]

What’s the benefit of having the scope argument in all these ctors?

[rix0rrr]

So the consumer is being forced to make a conscious choice about making it scoped or nonscoped. You can't just forget to supply the constructor arg.

[eladb]

But what is the meaning of a scoped AwsRegion? Moreover why do we even need those as separate classes? Aren’t then static properties sufficient?

I feel I am missing something :-)

[rix0rrr]

It is fundamentally correct to treat region as scoped because it depends on the providing stack, but practically useless because in the only situation in which you would return the value and can usefully use it, it would be the same as the consumer's AWS::Region.

So yeah, I'll change 😊

[eladb]

How does it make sense to export { AWS::StackName }? Or is it just to discover that things come from a different stack?

Something is oddd

[rix0rrr]

It could make sense:

Properties:
  Thing: 
    Type: AWS::Thing::Thing
Outputs:
  ThingStack:
    Value: { Ref: "AWS::StackName" }
    Export: 
      Name: TheStackWithTheThing

Creates an export with a well-known name pointing to the name under which this stack was ultimately deployed (does not necessarily need to be the name known to the CDK at deployment time).

Or do you mean, why can stackName be scoped? Because then the answer is, yes, to detect that it came from a different Stack object.

const stack1 = new StackWithThing(...);

const stack2 = new Stack(...);
new ThingConsumer(..., {
   thingStackName: stack1.stackName // would not be the right value if nonscoped
});

[eladb]

got it, thanks

[skinny85]

@rix0rrr looks good, but one question: how does this relate to the logic in cfn-tokens that disallows cross-region/account references? Or is that a separate change?

[skinny85]

@rix0rrr looks good, but one question: how does this relate to the logic in cfn-tokens that disallows cross-region/account references? Or is that a separate change?

Actually, I stumbled on this limitation very early working on my prototype, so I already started working on solving this issue. So scratch my comment.

[rix0rrr]

You should never get into that situation anymore

[rix0rrr]

The clou here is:

Either account and region were specified, in which case their literal values will be returned (so not a token, and no dependency will be registered), or they were not, in which case the consuming stack will be in the same situatin and the effective value for {AWS::Region} would be the same anyway.

[skinny85]

Sorry, I don't follow. You're saying the check that throws the Error in cfn-tokens is now dead code?

[rix0rrr]

Sorry, I don't follow. You're saying the check that throws the Error in cfn-tokens is now dead code?

Sorry, I see how this is confusing.

Tokens representing intrinsics (such as {Ref} and {Fn::GetAtt}) can be scoped or nonscoped. If they're scoped and used in a different stack, this situation will be detected and the producing stack will get to contain an Export and the consuming stack a {Fn::ImportValue}. This will only happen for stacks that share an account and region, because otherwise {Fn::ImportValue} won't be able to find the Export anyway. Other connections are disallowed.

For nonscoped intrinsics, nothing happens, and the {Ref} just gets rendered literally into the consuming stack.

It used to be that {Ref: "AWS::Region"} and similar, which are used when constructing ARNs from components, were scoped, so that they would be exported and imported if they were used in a cross-stack manner. That is a little silly because if the region was (say) us-east-1 then the producing stack would export AutomaticallyExportedToken: us-east-1 and the consuming stack would dutifully import this value, EXCEPT that this export/import would only work if both stacks would be in us-east-1 anyway. I've changed them to be nonscoped, so that the consuming stack will simply render the {Ref: AWS::Region} directly, because it would render to the same value anyway.

You're right that we're now missing an error case in case you refer to either the region or account of a stack that's in a different region. This used to error, and it will now no longer error. I think/hope this will not be a big problem. If we thought this was going to lead to problems, we could also make a Token class that counts as scoped for the purposes of the region/account check, but still renders to a non-export/imported value.

The other change is that if you refer to an other stack's fully-constructed ARN now, it won't register a stack dependency. That is actually required if we want bidirectional stack references such as you plan to do in your CI/CD work, so I don't know how to fix that. We could add in another exception that we do add a dependency for in-environment stacks but don't add a dependency for cross-environment stacks, but every new special case makes me a little sicker...