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

cfn2ts: some property types are parsed as interfaces instead of structs #6643

Closed
KyleMuellerPFG opened this issue Mar 9, 2020 · 22 comments
Closed
Labels
@aws-cdk/aws-wafv2 @aws-cdk/cfnspec bug This issue is a bug. effort/small Small work item – less than a day of effort jsii This issue originates in jsii, or this feature must be implemented in jsii. p1

Comments

@KyleMuellerPFG
Copy link

KyleMuellerPFG commented Mar 9, 2020

Using the python cdk, attempting to create a Web ACL with an IP whitelisting rule, but found that creating Web ACL rule properties does not work when trying to define an IP Set. I receive an error stating:
TypeError: Protocols cannot be instantiated

Reproduction Steps

       # Working rule prop, with aws managed rule group
       rule_sql = wafv2.CfnWebACL.RuleProperty(
            name='AWS-AWSManagedRulesSQLRuleSet',
            statement=wafv2.CfnWebACL.StatementOneProperty(
                managed_rule_group_statement=wafv2.CfnWebACL.ManagedRuleGroupStatementProperty(
                    vendor_name='AWS',
                    name='AWSManagedRulesSQLRuleSet'
                )
            ),
            visibility_config=wafv2.CfnWebACL.VisibilityConfigProperty(
                sampled_requests_enabled=True,
                cloud_watch_metrics_enabled=True,
                metric_name='AWS-AWSManagedRulesSQLRuleSet'
            ),
            priority=0,
        )

        # Non-working rule prop, with IP set reference
        rule_ip = wafv2.CfnWebACL.RuleProperty(
            name='IPRule',
            statement=wafv2.CfnWebACL.StatementOneProperty(
                ip_set_reference_statement=wafv2.CfnWebACL.IPSetReferenceStatementProperty(
                    arn='myArn'
                )
            ),
            visibility_config=wafv2.CfnWebACL.VisibilityConfigProperty(
                sampled_requests_enabled=True,
                cloud_watch_metrics_enabled=True,
                metric_name='IPRule'
            ),
            priority=0,
        )

Error Log

When synthing templates, this error is received:

File "...\infra\cdk.env\lib\site-packages\jsii_runtime.py", line 66, in call
inst = super().call(*args, **kwargs)
File "...\infra\cdk\stacks\stacks_lib\static_site_hosting.py", line 56, in init
arn='myArn'
File "...\infra\cdk.env\lib\site-packages\typing_extensions.py", line 1545, in _no_init
raise TypeError('Protocols cannot be instantiated')
TypeError: Protocols cannot be instantiated

Environment

  • CLI Version :
  • Framework Version: 1.27
  • OS : Windows
  • Language : Python

Other

Linked somewhat to: #6056


This is 🐛 Bug Report

@KyleMuellerPFG KyleMuellerPFG added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 9, 2020
@KyleMuellerPFG
Copy link
Author

As a workaround, I was able to implement the IPSetReferenceStatementProperty interface myself in python like so:

        @jsii.implements(wafv2.CfnRuleGroup.IPSetReferenceStatementProperty)
        class IPSetReferenceStatement:
            @property
            def arn(self):
                return self._arn

            @arn.setter
            def arn(self, value):
                self._arn = value

And then reference that new class instead:

        ip_set_ref_stmnt = IPSetReferenceStatement()
        ip_set_ref_stmnt.arn = whitelisted_ip_set.attr_arn

        # Non-working rule prop, with IP set reference
        whitelisted_ip_set_rule = wafv2.CfnWebACL.RuleProperty(
            name='AllowedIPsRule',
            statement=wafv2.CfnWebACL.StatementOneProperty(
                ip_set_reference_statement=ip_set_ref_stmnt
            ),
            visibility_config=wafv2.CfnWebACL.VisibilityConfigProperty(
                sampled_requests_enabled=True,
                cloud_watch_metrics_enabled=True,
                metric_name='AllowedIPsRule'
            ),
            priority=0,
        )

At which point, I am able to CDK synth the template, and it captures the proper reference to the IP Set Reference Statement.

@SomayaB SomayaB added the @aws-cdk/aws-waf Related to AWS Web Application Firewall label Mar 10, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 11, 2020

Ha!

Because of JSII naming conventions, IPSetReferenceStatementProperty will be interpreted as an interface, where it actually should be interpreted as a struct.

Our cfn2ts tool should have converted the name to IpSet (and probably WebACL => WebAcl).

@rix0rrr rix0rrr changed the title wafv2: IP Set Reference not working cfn2ts: some property types are parsed as interfaces instead of structs Mar 11, 2020
@rix0rrr rix0rrr added the p1 label Mar 11, 2020
@rix0rrr rix0rrr assigned eladb and unassigned RomainMuller and MrArnoldPalmer Mar 11, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 11, 2020

In fact it should be an error if any of the L1 generators generate any class that JSII will interpret as an interface. It should be all classes and structs, no?

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Mar 13, 2020
@eladb
Copy link
Contributor

eladb commented Mar 16, 2020

@SomayaB SomayaB added package/aws-wafv2 and removed @aws-cdk/aws-waf Related to AWS Web Application Firewall labels Mar 20, 2020
@praddc
Copy link

praddc commented Mar 23, 2020

Since I'm still having issues with #6056, could this be related?

@vishal0511
Copy link

I am also facing this issue with error message TypeError: Protocols cannot be instantiated when using ipset_reference_statement=waf.CfnRuleGroup.IPSetReferenceStatementProperty(arn="arn:XXXXXXXXX".
IS there any ETA when this will be resolved.

@eladb eladb added effort/small Small work item – less than a day of effort @aws-cdk/cfnspec labels Aug 4, 2020
@eladb eladb assigned rix0rrr and unassigned eladb Aug 17, 2020
@SomayaB SomayaB assigned shivlaks and unassigned rix0rrr Aug 20, 2020
@guysqr
Copy link

guysqr commented Nov 7, 2020

As a workaround, I was able to implement the IPSetReferenceStatementProperty interface myself in python like so:

        @jsii.implements(wafv2.CfnRuleGroup.IPSetReferenceStatementProperty)
        class IPSetReferenceStatement:
            @property
            def arn(self):
                return self._arn

            @arn.setter
            def arn(self, value):
                self._arn = value

And then reference that new class instead:

        ip_set_ref_stmnt = IPSetReferenceStatement()
        ip_set_ref_stmnt.arn = whitelisted_ip_set.attr_arn

        # Non-working rule prop, with IP set reference
        whitelisted_ip_set_rule = wafv2.CfnWebACL.RuleProperty(
            name='AllowedIPsRule',
            statement=wafv2.CfnWebACL.StatementOneProperty(
                ip_set_reference_statement=ip_set_ref_stmnt
            ),
            visibility_config=wafv2.CfnWebACL.VisibilityConfigProperty(
                sampled_requests_enabled=True,
                cloud_watch_metrics_enabled=True,
                metric_name='AllowedIPsRule'
            ),
            priority=0,
        )

At which point, I am able to CDK synth the template, and it captures the proper reference to the IP Set Reference Statement.

Thanks @KyleMuellerPFG - this enabled me to get an IP set working!

If anyone else finds this issue, note too that your IP rule can't contain the override_action parameter, or you'll get the error "Error reason: Your statement has multiple values set for a field that requires exactly one value., field: RULE," - because this is only valid for statements that refer to rule groups.

@singergs
Copy link

As a workaround, I was able to implement the IPSetReferenceStatementProperty interface myself in python like so:

        @jsii.implements(wafv2.CfnRuleGroup.IPSetReferenceStatementProperty)
        class IPSetReferenceStatement:
            @property
            def arn(self):
                return self._arn

            @arn.setter
            def arn(self, value):
                self._arn = value

And then reference that new class instead:

        ip_set_ref_stmnt = IPSetReferenceStatement()
        ip_set_ref_stmnt.arn = whitelisted_ip_set.attr_arn

        # Non-working rule prop, with IP set reference
        whitelisted_ip_set_rule = wafv2.CfnWebACL.RuleProperty(
            name='AllowedIPsRule',
            statement=wafv2.CfnWebACL.StatementOneProperty(
                ip_set_reference_statement=ip_set_ref_stmnt
            ),
            visibility_config=wafv2.CfnWebACL.VisibilityConfigProperty(
                sampled_requests_enabled=True,
                cloud_watch_metrics_enabled=True,
                metric_name='AllowedIPsRule'
            ),
            priority=0,
        )

At which point, I am able to CDK synth the template, and it captures the proper reference to the IP Set Reference Statement.

Thanks @KyleMuellerPFG - this enabled me to get an IP set working!

If anyone else finds this issue, note too that your IP rule can't contain the override_action parameter, or you'll get the error "Error reason: Your statement has multiple values set for a field that requires exactly one value., field: RULE," - because this is only valid for statements that refer to rule groups.

Just saw your fix. Where did you implement the workaround. Not sure where you make the code modification in CDK?

I ran into the same issue today. Thanks for posting.
GS

@NGL321 NGL321 assigned rix0rrr and njlynch and unassigned shivlaks Jan 25, 2021
@CarlosDomingues
Copy link

Was bitten by this today, thanks a lot @KyleMuellerPFG!

@RishiKapadia20
Copy link

Same here, wasted a lot of time on this. I thought CFN L1 structs were as good as cloudformation. Please get this fix in. Big thanks as well @KyleMuellerPFG

@dcarrillo
Copy link

Hi, same issue here, it took a lot of time to get here, please fix this, thanks so much @KyleMuellerPFG

@njlynch
Copy link
Contributor

njlynch commented Apr 29, 2021

See also #14445, where this was reported in Java as well.

@huyluudinh
Copy link

with aws_cdk 1.104.0, the "fix" doesnt work any more. I am getting this error:
jsii.errors.JSIIError: Resolution error: Resolution error: Expected a string, got {"$jsii.byref":"@aws-cdk/core.Reference@10005"}..
Anyone managed to implement IPSetReferenceStatementProperty in newer cdk?

@LoganAvatar
Copy link

with aws_cdk 1.104.0, the "fix" doesnt work any more. I am getting this error:
jsii.errors.JSIIError: Resolution error: Resolution error: Expected a string, got {"$jsii.byref":"@aws-cdk/core.Reference@10005"}..
Anyone managed to implement IPSetReferenceStatementProperty in newer cdk?

This worked fine in 1.107.0!

@awsiv
Copy link

awsiv commented Aug 27, 2021

I also thought L1 constructs were a 1-1 mapping with Cloudformation :(

Is anyone aware of a list of other properties that might need similar workaround?

@ivanmarquescepsa
Copy link

ivanmarquescepsa commented Oct 26, 2021

I'm facing this issue also with CfnIPSet.IPSetDescriptorProperty in CDK V2rc25

@charlesxucheng
Copy link

I was using the workaround mentioned above by Kyle but it stopped working after I upgraded to CDK 2.5.0. As advised by AWS support, the following works in my case:

        ip_set_v4 = wafv2.CfnIPSet(
            scope,
            'IPSetv4',
            addresses=Configuration.waf_ipsets,
            ip_address_version='IPV4',
            scope=WafProvisioner.__SCOPE_CLOUDFRONT,
        )
# ...
        web_acl = wafv2.CfnWebACL(
# ...
            rules=[
                wafv2.CfnWebACL.RuleProperty(
# ...
                    statement=wafv2.CfnWebACL.StatementProperty(
                        ip_set_reference_statement={
                            "arn": ip_set_v4.attr_arn
                        }
                    ),
                ),

@WingsLikeEagles
Copy link

I was using the workaround mentioned above by Kyle but it stopped working after I upgraded to CDK 2.5.0. As advised by AWS support, the following works in my case:

        ip_set_v4 = wafv2.CfnIPSet(
            scope,
            'IPSetv4',
            addresses=Configuration.waf_ipsets,
            ip_address_version='IPV4',
            scope=WafProvisioner.__SCOPE_CLOUDFRONT,
        )
# ...
        web_acl = wafv2.CfnWebACL(
# ...
            rules=[
                wafv2.CfnWebACL.RuleProperty(
# ...
                    statement=wafv2.CfnWebACL.StatementProperty(
                        ip_set_reference_statement={
                            "arn": ip_set_v4.attr_arn
                        }
                    ),
                ),

Can you give a little more info on how that is suppose to be implemented. Maybe some comments on the lines that are specific to your environment?
Is this line the name of your IP Set?
'IPSetv4,'
I'll go look at the docs for CfnIPSet, but a little more info would be helpful if you have the time.

@WingsLikeEagles
Copy link

I get this error when I try @charlesxucheng 's solution.
"ipSetReferenceStatement":{"$jsii.map":{"arn":"${Token[TOKEN.204]}"}}

@charlesxucheng
Copy link

I was using the workaround mentioned above by Kyle but it stopped working after I upgraded to CDK 2.5.0. As advised by AWS support, the following works in my case:

        ip_set_v4 = wafv2.CfnIPSet(
            scope,
            'IPSetv4',
            addresses=Configuration.waf_ipsets,
            ip_address_version='IPV4',
            scope=WafProvisioner.__SCOPE_CLOUDFRONT,
        )
# ...
        web_acl = wafv2.CfnWebACL(
# ...
            rules=[
                wafv2.CfnWebACL.RuleProperty(
# ...
                    statement=wafv2.CfnWebACL.StatementProperty(
                        ip_set_reference_statement={
                            "arn": ip_set_v4.attr_arn
                        }
                    ),
                ),

Can you give a little more info on how that is suppose to be implemented. Maybe some comments on the lines that are specific to your environment? Is this line the name of your IP Set? 'IPSetv4,' I'll go look at the docs for CfnIPSet, but a little more info would be helpful if you have the time.

Hi @WingsLikeEagles. Yes 'IPSetv4' is the id of the construct. waf_ipsets is an array of strings of CIDR ranges you want to attach to the rule.

@peterwoodworth peterwoodworth added the jsii This issue originates in jsii, or this feature must be implemented in jsii. label Jun 28, 2022
@TheRealAmazonKendra
Copy link
Contributor

It looks like this issue was fixed by upgrading to >2.5.0. I'm going to go ahead and close this one. If you believe this was closed in error or are using this version or above and are still experiencing this issue, please open a new issue.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-wafv2 @aws-cdk/cfnspec bug This issue is a bug. effort/small Small work item – less than a day of effort jsii This issue originates in jsii, or this feature must be implemented in jsii. p1
Projects
None yet
Development

No branches or pull requests