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

Acessing the 'DistributionConfig' of the 'CfnDistribution_' throws an error #3284

Closed
1 of 5 tasks
McDoit opened this issue Jul 11, 2019 · 22 comments
Closed
1 of 5 tasks
Labels
bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort language/dotnet Related to .NET bindings p1

Comments

@McDoit
Copy link
Contributor

McDoit commented Jul 11, 2019

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

  • I'm submitting a ...

    • 🪲 bug report
    • 🚀 feature request
    • 📚 construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    If the current behavior is a 🪲bug🪲: Please provide the steps to reproduce

Create a new CloudFrontWebDistribution, var cf = new CloudFrontWebDistribution
Access the underlying CfnDistribution_ with var cfd = cf.Node.FindChild("CFDistribution") as CfnDistribution_;
Acessing the DistributionConfig of the CfnDistribution_ throws an error:

Got 'undefined' for non-optional instance of {"abstract":true,"docs":{"remarks":"If this returns an empty array the stack will not be attached.","stability":"stable","summary":"The creation stack of this resolvable which will be appended to errors thrown during resolution."},"immutable":true,"locationInModule":{"filename":"lib/resolvable.ts","line":38},"name":"creationStack","type":{"collection":{"elementtype":{"primitive":"string"},"kind":"array"}}}
  • What is the expected behavior (or behavior of feature suggested)?

To be able to see the DistributionConfig of the CfnDistribution_

  • What is the motivation / use case for changing the behavior or adding this feature?

I was trying to work around other bugs in the CloudFrontWebDistribution by accessing the DistributionConfig to modify and add it back as overrides

  • Please tell us about your environment:

    • CDK CLI Version: 0.39
    • Module Version: 0.39
    • OS: [Windows 10]
    • Language: [C#]
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)
    Stacktrace:

Got 'undefined' for non-optional instance of {"abstract":true,"docs":{"remarks":"If this returns an empty array the stack will not be attached.","stability":"stable","summary":"The creation stack of this resolvable which will be appended to errors thrown during resolution."},"immutable":true,"locationInModule":{"filename":"lib/resolvable.ts","line":38},"name":"creationStack","type":{"collection":{"elementtype":{"primitive":"string"},"kind":"array"}}}

   at Amazon.JSII.Runtime.Services.Client.TryDeserialize[TResponse](String responseJson)
   at Amazon.JSII.Runtime.Services.Client.ReceiveResponse[TResponse]()
   at Amazon.JSII.Runtime.Deputy.DeputyBase.GetPropertyCore[T](JsiiPropertyAttribute propertyAttribute, Func`2 getFunc)
@McDoit McDoit added the needs-triage This issue or PR still needs to be triaged. label Jul 11, 2019
@RomainMuller
Copy link
Contributor

Does this problem still exist with cdk >= 1.0.0?

@McDoit
Copy link
Contributor Author

McDoit commented Jul 24, 2019

Still get the same error message in 1.1.0-devpreview

@McDoit
Copy link
Contributor Author

McDoit commented Aug 15, 2019

Still get the same error in 1.4-devpreview

@RomainMuller RomainMuller added language/dotnet Related to .NET bindings and removed needs-triage This issue or PR still needs to be triaged. labels Aug 26, 2019
@RomainMuller RomainMuller self-assigned this Aug 26, 2019
@RomainMuller RomainMuller added bug This issue is a bug. needs-reproduction This issue needs reproduction. and removed needs-response labels Aug 26, 2019
@McDoit
Copy link
Contributor Author

McDoit commented Aug 29, 2019

Here is a samll repro:

public class ReproCloudFrontStack : Stack
        {
            public ReproCloudFrontStack(Construct scope, string name, IStackProps props) : base(scope, name, props)
            {
                var cf = new CloudFrontWebDistribution(this, "MyDistribution", new CloudFrontWebDistributionProps
                {
                    HttpVersion = HttpVersion.HTTP2,
                    PriceClass = PriceClass.PRICE_CLASS_ALL,
                    OriginConfigs = new ISourceConfiguration[]
                        {
                            new SourceConfiguration
                            {
                                CustomOriginSource = new CustomOriginConfig
                                {
                                    DomainName = "example.com",
                                    HttpPort = 80,
                                    OriginProtocolPolicy = OriginProtocolPolicy.HTTP_ONLY,
                                },
                               Behaviors = new IBehavior[]
                               {
                                   new Behavior
                                   {
                                       AllowedMethods = CloudFrontAllowedMethods.ALL,
                                       Compress = true,
                                       IsDefaultBehavior = true
                                   }
                               }
                            }
                        }
                });

                var cfd = cf.Node.FindChild("CFDistribution") as CfnDistribution;

                var dcp = cfd.DistributionConfig as CfnDistribution.DistributionConfigProperty;

                var ir = cfd.DistributionConfig as IIResolvable;

                Console.WriteLine(ir.CreationStack);
            }
        }

cdp is null as DistributionConfig can't be resolved into CfnDistribution.DistributionConfigProperty

But casting DistributionConfig into a IIResolvable will make the ir.CreationStack throw the error

@McDoit
Copy link
Contributor Author

McDoit commented Sep 18, 2019

Still an issue with CDK 1.8 and JSII 0.17 sadly

@McDoit
Copy link
Contributor Author

McDoit commented Oct 2, 2019

Still an issue with CDK 1.10.1 and JSII 0.18 sadly

@RomainMuller RomainMuller removed the needs-reproduction This issue needs reproduction. label Oct 3, 2019
@RomainMuller
Copy link
Contributor

Alright - thanks for providing the repro code. We'll be having a look into the cause of this (hopefully) shortly, and will update the issue with our findings!

@McDoit
Copy link
Contributor Author

McDoit commented Oct 11, 2019

Perfect, thank you @RomainMuller!

These two are a bit related, at least the first, and might be beneficial to work on at the same time?

#3281
#3282

@RomainMuller
Copy link
Contributor

Sorry it took me so long to come back. I'm looking into testing your reproduction against the current HEAD of aws/jsii, in hopes that the last kernel bug I fixed was the cause of this issue (I'm pretty optimistic!)

@McDoit
Copy link
Contributor Author

McDoit commented Nov 2, 2019

No worries

Just tried the latest CDK with JSII 0.19 with no change
With JSII 0.20 and Visual studio 2019 i get a new error:
Unhandled exception. System.MissingMethodException: .ctor
I guess i should file that as a new issue

As a side note i get the following erro in my full code when using alias config and a ACM cert:
Unhandled Exception: Amazon.JSII.Runtime.JsiiException: acmCertificate certficate must be in the us-east-1 region, got eu-west-1
even if i specify "us-east-1" in the DnsValidatedCertificate i setup in the stack

@RomainMuller
Copy link
Contributor

Yeah the problem is our dependencies are currently not modeled currently and this allowed an incompatible JSII runtime to be picked up by your install. We are going to release a fixed version very soon.

@McDoit
Copy link
Contributor Author

McDoit commented Nov 22, 2019

Just tested this again, no error this time, but the object returned cant be cast into a DistributionConfigProperty, it returns a Amazon.JSII.Runtime.Deputy.AnonymousObject instead

It throws this:

System.InvalidCastException: 'Unable to cast object of type 'Amazon.JSII.Runtime.Deputy.AnonymousObject' to type 'DistributionConfigProperty'.'

@McDoit
Copy link
Contributor Author

McDoit commented Jan 16, 2020

@RomainMuller is it possible to open this one up again, as it is still not fixed?

@njlynch
Copy link
Contributor

njlynch commented Oct 16, 2020

I verified; just reproduced the above errors (Amazon.JSII.Runtime.Deputy.AnonymousObject/NPE) on CDK 1.68.0.

@njlynch njlynch reopened this Oct 16, 2020
@McDoit
Copy link
Contributor Author

McDoit commented Oct 27, 2020

@njlynch Maybe unassign @RomainMuller so someone else can take a look?

@RomainMuller
Copy link
Contributor

The issue is that cfd.DistributionConfig is originally typed as a union, which prevents the .NET code from having enough type information to systematically back this cast. We'll probably need to offer a separate API to allow for this (one already exists, but it might still not allow what you are trying to do right now -- although it probably can be modified to allow that).

@njlynch
Copy link
Contributor

njlynch commented Oct 27, 2020

@McDoit ,

Separate from the underlying issue, I wonder if we can help with anything with the construct library itself. What is it that CloudFront is missing such that you need to resort to these escape hatches and post-creation modifications?

RomainMuller added a commit to aws/jsii that referenced this issue Oct 27, 2020
In order to allow converting an opaque object instance to an arbitrary
(jsii-managed) interface type, the `UnsafeCase` operation may be used to
instantiate a proxy while bypassing all type consistency checks.

This is similar to using `as any` or `as T` in TypeScript, meaning that
if the user performs a cast to an incorrect/unsupported type, undefined
behavior follows.

This would unblock certain specific use-case scenarios that static
languages render difficult to enact, such as the one described in
aws/aws-cdk#3284.
@RomainMuller
Copy link
Contributor

RomainMuller commented Oct 27, 2020

I've created aws/jsii#2192 to introduce an UnsafeCast<T>() method on DeputyBase (the parent class of AnonymousObject) to allow casting to an arbitrary interface type.

This would allow you to write something along the lines of:

var dcp = (cfd.DistributionConfig as DeputyBase).UnsafeCast<CfnDistribution.DistributionConfigProperty>();

Effectively un-blocking your use-case (and other use-cases that involve property overrides with complex/composite properties).

This might become more elegant in the future, as type unions are supported better (more info at aws/aws-cdk-rfcs#193).

mergify bot pushed a commit to aws/jsii that referenced this issue Oct 28, 2020
In order to allow converting an opaque object instance to an arbitrary
(jsii-managed) interface type, the `UnsafeCase` operation may be used to
instantiate a proxy while bypassing all type consistency checks.

This is similar to using `as any` or `as T` in TypeScript, meaning that
if the user performs a cast to an incorrect/unsupported type, undefined
behavior follows.

This would unblock certain specific use-case scenarios that static
languages render difficult to enact, such as the one described in
aws/aws-cdk#3284.

In particular, this is the only way out until aws/aws-cdk-rfcs#193
is delivered, for dealing with instances of interfaces returned through
a type union API.

---

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

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@McDoit
Copy link
Contributor Author

McDoit commented Oct 29, 2020

@McDoit ,

Separate from the underlying issue, I wonder if we can help with anything with the construct library itself. What is it that CloudFront is missing such that you need to resort to these escape hatches and post-creation modifications?

Trying to remember it from over a year ago, but I think the reason was to modify some option and naming to convert our existing CF stacks to using the CDK instead. So deploying the same stack name (and resources) but wrap it in the CDK instead.

Main issue there was that replacing the CF distributions as custom domains on them need to be unique, and we didn't want to deal with it manually or have downtime solving it.

@McDoit
Copy link
Contributor Author

McDoit commented Oct 29, 2020

I've created aws/jsii#2192 to introduce an UnsafeCast<T>() method on DeputyBase (the parent class of AnonymousObject) to allow casting to an arbitrary interface type.

This would allow you to write something along the lines of:

var dcp = (cfd.DistributionConfig as DeputyBase).UnsafeCast<CfnDistribution.DistributionConfigProperty>();

Effectively un-blocking your use-case (and other use-cases that involve property overrides with complex/composite properties).

This might become more elegant in the future, as type unions are supported better (more info at aws/aws-cdk-rfcs#193).

I tried it with CDK 1.70 and JSII 1.14, but I got the following error running the repro stack above:
System.InvalidCastException: 'Unable to cast Amazon.JSII.Runtime.Deputy.AnonymousObject into Amazon.CDK.AWS.CloudFront.CfnDistribution+DistributionConfigProperty'

@RomainMuller
Copy link
Contributor

Hmmm. Thanks for reporting. I'll have a look into it.

@RomainMuller RomainMuller added the effort/medium Medium work item – several days of effort label Dec 3, 2020
@RomainMuller RomainMuller removed their assignment Jun 21, 2021
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort language/dotnet Related to .NET bindings p1
Projects
None yet
Development

No branches or pull requests

3 participants