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

Permanent diff on aws.wafv2.RuleGroup #3190

Closed
t0yv0 opened this issue Dec 27, 2023 · 3 comments
Closed

Permanent diff on aws.wafv2.RuleGroup #3190

t0yv0 opened this issue Dec 27, 2023 · 3 comments
Assignees
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed service/wafv2 Issues with aws.wafv2 resources

Comments

@t0yv0
Copy link
Member

t0yv0 commented Dec 27, 2023

What happened?

Calling pulumi up repeatedly produces non-empty diff.

Example

// Copyright 2016-2023, Pulumi Corporation.

import * as pulumi from "@pulumi/pulumi";
import * as aws from "@pulumi/aws";

const config = new pulumi.Config("aws");
const providerOpts = { provider: new aws.Provider("prov", { region: <aws.Region>config.require("envRegion") }) };

new aws.wafv2.RuleGroup("waf", {
    description: "RuleGroup for general purposes",
    scope: "REGIONAL",
    visibilityConfig: {
        cloudwatchMetricsEnabled: false,
        metricName: "metric",
        sampledRequestsEnabled: false,
    },
    capacity: 500,
    rules: [{
        name: 'rule-1',
        action: {
            block: {},
        },
        priority: 1,
        statement: {
            notStatement: {
                statements: [{
                    andStatement: {
                        statements: [
                            {
                                geoMatchStatement: {
                                    countryCodes: ["US"]
                                },
                            },
                            {
                                byteMatchStatement: {
                                    positionalConstraint: "CONTAINS",
                                    searchString: "word",
                                    fieldToMatch: {
                                        allQueryArguments: {}
                                    },
                                    textTransformations: [{
                                        priority: 5,
                                        type: "CMD_LINE",
                                    }, {
                                        priority: 2,
                                        type: "LOWERCASE",
                                    }],
                                },
                            },
                        ],
                    },
                }],
            },
        },
        visibilityConfig: {
            cloudwatchMetricsEnabled: false,
            metricName: 'rule-1',
            sampledRequestsEnabled: false,
        },
    }],
}, providerOpts);

The example is in examples/wafv2.

Output of pulumi about

CLI
Version 3.97.0
Go Version go1.21.4
Go Compiler gc

Plugins
NAME VERSION
aws 5.43.0
nodejs unknown

Host
OS darwin
Version 14.1.1
Arch x86_64

This project is written in nodejs: executable='/Users/t0yv0/bin/node' version='v18.18.2'

Current Stack: t0yv0/WafV2/wafv2awstest

TYPE URN
pulumi:pulumi:Stack urn:pulumi:wafv2awstest::WafV2::pulumi:pulumi:Stack::WafV2-wafv2awstest
pulumi:providers:aws urn:pulumi:wafv2awstest::WafV2::pulumi:providers:aws::prov
aws:wafv2/ruleGroup:RuleGroup urn:pulumi:wafv2awstest::WafV2::aws:wafv2/ruleGroup:RuleGroup::waf

Found no pending operations associated with wafv2awstest

Backend
Name pulumi.com
URL https://app.pulumi.com/t0yv0
User t0yv0
Organizations t0yv0, pulumi
Token type personal

Dependencies:
NAME VERSION
@pulumi/aws 5.43.0
@pulumi/pulumi 3.99.0
@types/aws-sdk 2.7.0
@types/node 8.10.66

Pulumi locates its logs in /var/folders/gk/cchgxh512m72f_dmkcc3d09h0000gp/T/com.apple.shortcuts.mac-helper// by default

Additional context

This example was tested but for some historic reasons it was tested under Quick: True option of ProgramTest that disabled the checks that verify against repeated diffs.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@t0yv0 t0yv0 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Dec 27, 2023
@iwahbe iwahbe added bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. and removed needs-triage Needs attention from the triage team labels Dec 28, 2023
@t0yv0 t0yv0 added the service/wafv2 Issues with aws.wafv2 resources label May 3, 2024
@t0yv0
Copy link
Member Author

t0yv0 commented May 6, 2024

    ~ aws:wafv2/ruleGroup:RuleGroup: (update)
        [id=636067c5-d5de-406c-a09a-7fa1848b2cdd]
        [urn=urn:pulumi:dev::aws-3190::aws:wafv2/ruleGroup:RuleGroup::waf]
        [provider=urn:pulumi:dev::aws-3190::pulumi:providers:aws::prov::599aac36-1f59-4df1-906a-42f23a95f15b]
      ~ rules: [
          ~ [0]: {
                  ~ action          : {
                      + __defaults: []
                      ~ block     : {
                          + __defaults: []
                        }
                    }
                  ~ name            : "rule-1" => "rule-1"
                  ~ priority        : 1 => 1
                  ~ statement       : {
                      + __defaults  : []
                      ~ notStatement: {
                          + __defaults: []
                          ~ statements: [
                              ~ [0]: {
                                      + __defaults  : []
                                      ~ andStatement: {
                                          + __defaults: []
                                          ~ statements: [
                                              ~ [0]: {
                                                      + __defaults       : []
                                                      ~ geoMatchStatement: {
                                                          + __defaults  : []
                                                            countryCodes: [
                                                                [0]: "US"
                                                            ]
                                                        }
                                                    }
                                              ~ [1]: {
                                                      + __defaults        : []
                                                      ~ byteMatchStatement: {
                                                          + __defaults          : []
                                                          ~ fieldToMatch        : {
                                                              + __defaults       : []
                                                              ~ allQueryArguments: {
                                                                  + __defaults: []
                                                                }
                                                              - headerOrders     : []
                                                              - headers          : []
                                                            }
                                                            positionalConstraint: "CONTAINS"
                                                          ~ searchString        : "word" => "word"
                                                          ~ searchString        : "word" => "word"
                                                          ~ textTransformations : [
                                                              ~ [0]: {
                                                                      + __defaults: []
                                                                      ~ priority  : 2 => 5
                                                                      ~ type      : "LOWERCASE" => "CMD_LINE"
                                                                    }
                                                              ~ [1]: {
                                                                      + __defaults: []
                                                                      ~ priority  : 5 => 2
                                                                      ~ type      : "CMD_LINE" => "LOWERCASE"
                                                                    }
                                                            ]
                                                        }
                                                    }
                                            ]
                                        }
                                    }
                            ]
                        }
                    }
                  ~ visibilityConfig: {
                      + __defaults              : []
                        cloudwatchMetricsEnabled: false
                        metricName              : "rule-1"
                        sampledRequestsEnabled  : false
                    }
                }
        ]

This is still a problem; it looks very similar to pulumi/pulumi-terraform-bridge#1917 - we have a confused diff because the set element hash is perceived to be changing, and the reason for this is nil/empty array confusion:

       - headerOrders     : []
       - headers          : []

@t0yv0
Copy link
Member Author

t0yv0 commented May 6, 2024

If this is the right call, something like this #3897 could be used to work around if we cannot fix the root cause in the bridge.

t0yv0 added a commit to pulumi/pulumi-terraform-bridge that referenced this issue May 6, 2024
With AWS 3880 there is some evidence (derivation in
#1917) that
sometimes TF has entries in the InstanceDiff.Attributes while still
planning to take the resource to the end-state that is identical to the
original state. IN these cases, TF does not display a diff but Pulumi
does.

The root cause here remains unfixed
(#1895) - Pulumi
bridge is editing terraform-pulgin-sdk to expose the InstanceDiff
structure to connect it to the makeDetailedDiff machinery. Pulumi
should, like TF, stick to the gRPC protocol and rely only on the
PlannedState value.

We can incrementally approach the desired behavior with this change
though which detects PlannedState=PriorState case and suppresses any
diffs in this case.

Fixes:

- pulumi/pulumi-aws#3880
- pulumi/pulumi-aws#3306
- pulumi/pulumi-aws#3190
- pulumi/pulumi-aws#3454

---------

Co-authored-by: Venelin <venelin@pulumi.com>
This was referenced May 7, 2024
flostadler added a commit that referenced this issue May 15, 2024
WAFv2 RuleGroups used to have a perma diff for the rules property.
Enrolling the resource in PlanResourceChange fixes that.

Fixes #3306,
#3880,
#3190,
#3454
@flostadler flostadler self-assigned this May 15, 2024
@flostadler flostadler added the resolution/fixed This issue was fixed label May 15, 2024
@flostadler
Copy link
Contributor

This was fixed in #3948. It'll be released in release 6.36.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed service/wafv2 Issues with aws.wafv2 resources
Projects
None yet
Development

No branches or pull requests

3 participants