-
Notifications
You must be signed in to change notification settings - Fork 156
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
constant diff from wafv2 with no changes #1423
Comments
Is anyone from Pulumi looking into this? We're running into the same issues with the TypeScript sdk (@pulumi/aws 4.10.0): webAcl's diff always renders as if creating all the rules from scratch, even when there are no changes to apply. #1098 identified the same problem (among others) nearly a year ago, and the lack of activity on both issues leaves me worried that this behavior will continue to fall through the cracks indefinitely. |
I took a brief look and I can't figure out why this is happening. It seems to be some combination of: the way we check/diff old state versus new state, populate the inputs/outputs based on prior program runs, and/or, possibly, set diffing. Here is some raw data. I think someone like @pgavlin, @mikhailshilkov, or @EvanBoyle will need to weigh in, however. The Inputs:
Outputs:
Here is what we send over RPC during the Diff command, which perceives a difference: Olds: News: And here is the final determination resulting from that invocation of Diff, which decides there is a difference: |
Just hit this myself. Any workaround besides just updating each time? |
Running into this myself. Any news on this? |
Any news on this? |
Anyone has a workaround for this? |
Same here, any news? |
I was encountering this and switched to the AWS Native provider (just for wafv2, the rest of my stack is still using the AWS Classic provider), adjusted a few parameter names (different capitalization on a few things), and it doesn't have this issue. |
I'm also running into this issue and also tried the AWS native provider for this resource. That one has it's own issues that I didn't manage to workaround, which lead me to try this classic version. |
This is really a pain for us, running pulumi with the CI we have all this noise in the preview output each time. |
I can confirm we're also seeing this internally at Pulumi. I tested 3 versions of pulumi-aws (ts) which all show the bug -- 4.14, 4.28, and 5.9.2.
Sorry you're running into this. It's the kind of bug that degrades the usefulness of the CI integration, so it's top of mind for me. If we always show the user "hey check this out!" when in reality nothing is changing, it's less likely they'll notice or care when something actually does change. |
@blampe I would like to help solving this issue, did you already know what could be? Is the state that is not saved correctly? |
I'm not sure where the problem is. Last week I confirmed
|
Verified that this is still an issue and also does not seem to occur with an equivalent TF template. Going to check into the Diff behavior in the bridge. |
Can you paste the TF config here for posterity? |
@pgavlin Any commented out stuff is from me and @stack72 attempting to debug, but it should be pretty close to the original: terraform {
required_providers {
aws = {
source = "hashicorp/aws"
version = "~> 4.0"
}
}
}
# Configure the AWS Provider
provider "aws" {
region = "us-east-1"
}
resource "aws_wafv2_web_acl" "web_acl" {
name = "josh-test"
default_action {
block {}
}
scope = "REGIONAL"
visibility_config {
cloudwatch_metrics_enabled = false
metric_name = "josh-test"
sampled_requests_enabled = true
}
rule {
name = "josh-test-aws-rules"
override_action {
count {}
}
priority = 1
statement {
managed_rule_group_statement {
vendor_name = "AWS"
name = "AWSManagedRulesCommonRuleSet"
# version = "Version_1.3"
}
}
visibility_config {
cloudwatch_metrics_enabled = false
metric_name = "josh-test-aws-rules"
sampled_requests_enabled = true
}
}
tags = {
"key" = "value"
}
} And the equivalent Pulumi: import * as aws from "@pulumi/aws";
new aws.wafv2.WebAcl("web-acl", {
defaultAction: {
block: {},
},
scope: "REGIONAL",
visibilityConfig: {
cloudwatchMetricsEnabled: false,
metricName: "josh-test",
sampledRequestsEnabled: true,
},
rules: [{
name: "josh-test-aws-rules",
overrideAction: {
count: {},
},
priority: 1,
statement: {
managedRuleGroupStatement: {
vendorName: "AWS",
name: "AWSManagedRulesCommonRuleSet",
// version: "Version_1.3",
},
},
visibilityConfig: {
cloudwatchMetricsEnabled: false,
metricName: "josh-test-aws-rules",
sampledRequestsEnabled: true,
}
}]
}); @stack72 and I took a look at this for several hours today. We checked the TF provider schema, nothing doing. Here's some dumped olds and news, if it's any help:
Per @stack72, it looks like the program itself may not be correctly translating to state, but the TF schema does not appear to have anything ususual as far as we can tell. |
We've worked around this with: // NOTE: while updating "rules", you must comment out "ignoreChanges" setting.
// see https://github.com/pulumi/pulumi-aws/issues/1423
new aws.wafv2.WebAcl("xxx", {
rules: [ ... ],
...
}, {
ignoreChanges: ["rules"],
}); This is a bad workaround. |
In playing around with a test stack, this seems to in part be related to empty actions in rules. For instance For instance, this rule won't converge
But changing Unfortunately some actions are designed to only ever be empty, (e.g. For my own purposes, I switched to the aws-native provider for this resource because it also has better behavior wrt the |
any update on this one? |
The new pulumi/terraform-diff-reader v0.0.1 version has a workaround for pulumi/pulumi-aws#1423 and should fix it once the pulumi-aws provider upgrades to the new bridge version. A regression test is included.
The new pulumi/terraform-diff-reader v0.0.1 version has a workaround for pulumi/pulumi-aws#1423 and should fix it once the pulumi-aws provider upgrades to the new bridge version. A regression test is included.
#2604 will fix this. The change will be released as part of |
Unfortunately our users report the issue resurfacing under a slightly different configuration on v6 in #2664 I'm reopening for now. Reopening for now. We'll be targeting a patch release following the first v6 release to land a comprehensive fix. |
We believe we have a fix for this issue that's part of the v6 release candidate and will be released as pulumi-aws v6.x.x. I will close the issue once that's released. |
The fix should be available in v6.0.2, covering #2664 and other scenarios. Please let us know if there's any remaining problems around this! |
Hi @t0yv0, would it be possible to reopen the issue? EDIT: the regression on the |
Let's continue this discussion in #3454. |
The following wafv2 config...
produces this diff every time I run pulumi up. The update succeeds, with no changes as far as I can see.
requirements.txt
The text was updated successfully, but these errors were encountered: