cdktn: synthesis succeeds if resource is added with app's scope

[krzema12]

Expected Behavior

For resources attached to scope=App(), I'd expect CDKTF to either deploy it within some root/unnamed stack, or fail fast since this resource isn't created under any stack.

Actual Behavior

CDKTF succeeds during synthesis, and doesn't deploy the resource anywhere.

Steps to Reproduce

See this piece of code:

from cdktn import App
from cdktn_provider_google.project_iam_member import ProjectIamMember

ProjectIamMember(
    scope=app,
    id_="some-iam-member",
    member=...,
    project=...
    role=...
)

Versions

cdktn: 0.22.0
cdktn-cli: 0.22.0

Providers

cdktn-provider-google==17.0.0

Gist

No response

Possible Solutions

I don't know what's the proper solution yet, given that ideally we'd fail fast for all supported languages.

Workarounds

In my case, I use Python. I added extra validation as a subclass of App, like:

class ValidatedApp(App):
       def synth(self) -> None:
        orphans = []
        for node in self.node.find_all():
            if isinstance(node, (App, TerraformStack)):
                continue
            try:
                find_terraform_stack(node)
            except ValueError:
                orphans.append(Node.of(node).path)
        if orphans:
            bullet_list = "\n".join(f"  - {path}" for path in orphans)
            raise ValueError(
                "Found constructs outside of a TerraformStack "
                "(did you pass scope=app instead of scope=stack?):\n" + bullet_list
            )
        super().synth()

Anything Else?

No response

References

No response

Help Wanted

• I'm interested in contributing a fix myself

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

[so0k]

I prefer failfast. In TerraConstructs the base construct all my L2 extend forces scope validation (StackBase.ofTerraConstruct(this) in constructor will throw)

https://github.com/TerraConstructs/base/blob/d02464eee83d4a5f5d7b4350edd1cc5f1490489c/src/stack-base.ts#L169-L177

it seems neither TerraformResource and TerraformElement verify their scope.
There is:

cdk-terrain/packages/cdktn/lib/terraform-element.ts

Lines 47 to 49 in bacfe6e

	public get cdktfStack(): TerraformStack {
	return TerraformStack.of(this);
	}

which will throw if there it is not part of a stack

cdk-terrain/packages/cdktn/lib/terraform-stack.ts

Lines 114 to 124 in bacfe6e

	if (!node.scope) {
	let hint = "";
	if (construct.node.scope === c && App.isApp(c)) {
	// the scope of the originally passed construct equals the construct c
	// which has no scope (i.e. has no parent construct) and c is an App
	// and our construct is a Backend
	hint = `. You seem to have passed your root App as scope to a construct. Pass a stack (inheriting from TerraformStack) as scope to your construct instead.`;
	}
	throw noStackForConstruct(construct.node.path, hint);
	}

this is what my L2 constructs call and fail fast

if we change this in cdktn core, it will be a breaking change and could cause issues for many people tho

[so0k]

Given we can ensure the requested behaviour in our L2 implementations, and changing the CDKTN core behaviour would be a breaking change in either way.. I'm not sure we should implement this

[krzema12]

How about making it an opt-in behavior?

[so0k]

yes, we could introduce a feature flag for this

cdk-terrain/CONTRIBUTING.md

Lines 353 to 393 in 1afab72

	## Feature Flags
	Sometimes we want to introduce new breaking behavior because we believe this is
	the correct default behavior for the CDK Terrain. The problem of course is that breaking
	changes are only allowed in major versions and those are rare.
	To address this need, we have a feature flags pattern/mechanism. It allows us to
	introduce new breaking behavior which is disabled by default (so existing
	projects will not be affected) but enabled automatically for new projects
	created through `cdktn init`.
	The pattern is simple:
	1. Define a new const under
	[cdktn/lib/features.ts](https://github.com/open-constructs/cdk-terrain/blob/main/packages/cdktn/lib/features.ts)
	with the name of the context key that **enables** this new feature (for
	example, `EXCLUDE_STACK_ID_FROM_LOGICAL_IDS`).
	2. Use `node.tryGetContext(ENABLE_XXX)` to check if this feature is enabled
	in your code. If it is not defined, revert to the legacy behavior.
	3. Add your feature flag to the `FUTURE_FLAGS` map in
	[cdktn/lib/features.ts](https://github.com/open-constructs/cdk-terrain/blob/main/packages/cdktn/lib/features.ts).
	This map is inserted to generated `cdktf.json` files for new projects created
	through `cdktn init`.
	4. In your PR title (which goes into CHANGELOG), add a `(under feature flag)` suffix. e.g:
	```
	fix(core): top level constructs should omit stack id from name (under feature flag)
	```
	5. Under `BREAKING CHANGES` in your commit message describe this new behavior:
	```
	BREAKING CHANGE: top level resource names for new projects created through "cdktn init"
	will omit the stack id from their name. This is enabled through the flag
	`excludeStackIdFromLogicalIds` in newly generated `cdktf.json` files.
	```
	In the next major version of the
	CDKTN we will either remove the
	legacy behavior or flip the logic for all these features and then
	reset the `FEATURE_FLAGS` map for the next cycle.

[krzema12]

@so0k PTAL: #73 - adding a test first.