pulumi / pulumi-cloudflare

Pulumi's Cloudflare package, providing multi-language infrastructure as code for Cloudflare
Apache License 2.0
104 stars 13 forks source link

Importing RuleSet does not import rules and try to replace #554

Closed ami-descope closed 7 months ago

ami-descope commented 1 year ago

What happened?

I import a ruleset for WAF successfuly, now when I run pulumi up again it wants to replace it, but its exists so it fails

importing ❯ pulumi import cloudflare:index/ruleset:Ruleset domain-rate-limit zones/b0568fbe68169ce1aec3c6f82fca4d6b/be7027900a384d6b9a83d67ae4e13f83 --protect=false

  warning: One or more imported inputs failed to validate. This is almost certainly a bug in the `cloudflare` provider. The import will still proceed, but you will need to edit the generated code after copying it into your program.
    warning: cloudflare:index/ruleset:Ruleset resource 'domain-rate-limit' has a problem: Missing required property 'kind': Type of Ruleset to create. Available values: `custom`, `managed`, `root`, `zone`.
    warning: cloudflare:index/ruleset:Ruleset resource 'domain-rate-limit' has a problem: Missing required property 'phase': Point in the request/response lifecycle where the ruleset will be created. Available values: `ddos_l4`, `ddos_l7`, `http_config_settings`, `http_custom_errors`, `http_log_custom_fields`, `http_ratelimit`, `http_request_cache_settings`, `http_request_dynamic_redirect`, `http_request_firewall_custom`, `http_request_firewall_managed`, `http_request_late_transform`, `http_request_origin`, `http_request_redirect`, `http_request_sanitize`, `http_request_sbfm`, `http_request_transform`, `http_response_compression`, `http_response_firewall_managed`, `http_response_headers_transform`, `magic_transit`.
    warning: cloudflare:index/ruleset:Ruleset resource 'domain-rate-limit' has a problem: Missing required property 'name': Name of the ruleset.

Resources:
    = 1 to import

yes...

Resources:
    = 1 imported
    505 unchanged

Duration: 3s

Please copy the following code into your Pulumi application. Not doing so
will cause Pulumi to report that an update will happen on the next update command.

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

const domain_rate_limit = new cloudflare.Ruleset("domain-rate-limit", {
    accountId: "REDUCTED",
    kind: "",
    name: "",
    phase: "",
});

worth mentioning that the rules are empty when i check the state after import

 {
        "urn": "urn:pulumi:dev::main-apps::cloudflare:index/ruleset:Ruleset::domain-rate-limit",
        "custom": true,
        "id": "be7027900a384d6b9a83d67ae4e13f83",
        "type": "cloudflare:index/ruleset:Ruleset",
        "inputs": {
          "accountId": "b0568fbe68169ce1aec3c6f82fca4d6b"
        },
        "outputs": {
          "accountId": "REDUCTED",
          "id": "be7027900a384d6b9a83d67ae4e13f83",
          "rules": []
        },
        "parent": "urn:pulumi:dev::main-apps::pulumi:pulumi:Stack::main-apps-dev",
        "provider": "urn:pulumi:dev::main-apps::pulumi:providers:cloudflare::default_5_12_0::8637ac28-cfcd-4e30-b76f-26129f4484b9",
        "created": "2023-11-07T00:48:32.997955Z",
        "modified": "2023-11-07T00:48:32.997955Z"
      }

running pulumi up

+-cloudflare:index/ruleset:Ruleset: (replace)
        [id=be7027900a384d6b9a83d67ae4e13f83]
        [urn=urn:pulumi:dev::main-apps::cloudflare:index/ruleset:Ruleset::domain-rate-limit]
        [provider=urn:pulumi:dev::main-apps::pulumi:providers:cloudflare::default_5_12_0::8637ac28-cfcd-4e30-b76f-26129f4484b9]
      - accountId  : "[secret]"
      + description: output<string>
      ~ id         : "be7027900a384d6b9a83d67ae4e13f83" => output<string>
      + kind       : "zone"
      + name       : "domain-rate-limit"
      + phase      : "http_ratelimit"
      ~ rules      : [
          + [0]: {
                  + action     : "block"
                  + description: "default - 100/min"
                  + enabled    : true
                  + expression : "(ends_with(http.host, \"domain.com\") and http.host ne \"random.domain.com\" and not starts_with(http.request.uri.path, \"/.well-known\") and not starts_with(http.request.uri.path, \"/scim\") and not any(http.request.headers[\"x-sdk-go-version\"][*] contains \".\") and not any(http.request.headers[\"x-sdk-python-version\"][*] contains \".\") and not any(http.request.headers[\"x-sdk-node-version\"][*] contains \".\"))"
                  + id         : output<string>
                  + lastUpdated: output<string>
                  + ratelimit  : {
                      + characteristics  : [
                      +     [0]: "cf.unique_visitor_id"
                      +     [1]: "cf.colo.id"
                        ]
                      + mitigationTimeout: 60
                      + period           : 60
                      + requestsPerPeriod: 100
                      + requestsToOrigin : true
                    }
                  + ref        : output<string>
                  + version    : output<string>
                }
        ]
      + zoneId     : [secret]

Example

new cloudflare.Ruleset("domain-rate-limit", {
        name: "domain-rate-limit",
        kind: "zone",
        phase: "http_ratelimit",
        zoneId: config.cloudflareZoneId,
        rules: [
            {
                action: "block",
                ratelimit: {
                    characteristics: ["cf.unique_visitor_id", "cf.colo.id"],
                    period: 60,
                    requestsPerPeriod: 100,
                    mitigationTimeout: 60,
                    requestsToOrigin: true,
                },
                // expression: `(ends_with(http.host, ${config.zoneDomainName}) and http.host ne "console.${config.zoneDomainName})" and not starts_with(http.request.uri.path, "/.well-known") and not starts_with(http.request.uri.path, "/scim") and not any(http.request.headers["x-sdk-go-version"][*] contains ".") and not any(http.request.headers["x-sdk-python-version"][*] contains ".") and not any(http.request.headers["x-sdk-node-version"][*] contains "."))`,
                expression: `(ends_with(http.host, "domain.com") and http.host ne "random.domain.com" and not starts_with(http.request.uri.path, "/.well-known") and not starts_with(http.request.uri.path, "/scim") and not any(http.request.headers["x-sdk-go-version"][*] contains ".") and not any(http.request.headers["x-sdk-python-version"][*] contains ".") and not any(http.request.headers["x-sdk-node-version"][*] contains "."))`,
                description: "default - 100/min",
                enabled: true,
            },
        ],
    });

Output of pulumi about

pulumi about CLI Version 3.92.0 Go Version go1.21.3 Go Compiler gc

Plugins NAME VERSION nodejs unknown

Host OS darwin Version 13.6 Arch arm64

This project is written in nodejs: executable='/opt/homebrew/bin/node' version='v20.5.1'

Additional context

No response

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).

mikhailshilkov commented 11 months ago

Sorry for a long delay with response, I needed to change the setup of my Cloudflare test account.

I was able to reproduce the error. I used this sample program to provision a ruleset:

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

const example = new cloudflare.Zone("example", {
    accountId: "<account id>",
    zone: "test-mikhail-example.com",
});

new cloudflare.Ruleset("domain-rate-limit", {
    zoneId: example.id,
    name: "transform rule for URI path",
  description: "change the URI path to a new static path",
  kind: "zone",
  phase: "http_request_transform",

  rules: [{
    action: "rewrite",
    actionParameters: {
      uri: {
        path: {
          value: "/my-new-route"
        }
      }
    },

    expression: "(http.host eq \"example.com\" and http.request.uri.path eq \"/old-path\")",
    description: "example URI path transform rule",
    enabled: true,
  }]
});

and then imported the same ruleset. I got the same result. It looks like we get an almost entirely empty state from the Read operation:

Marshaling property for RPC[newState]: accountId={<account id>}
Marshaling property for RPC[newState]: id={<ruleset id>}
Marshaling property for RPC[newState]: rules={[]}

is all I can see.

Something is odd on our side here, needs further debugging. Note that this is a TFPF resource.

ami-descope commented 11 months ago

@mikhailshilkov here is the reponse i got from the terraform provider issue i opened

it's not a terraform issue.

https://github.com/cloudflare/terraform-provider-cloudflare/issues/2915

ami-descope commented 11 months ago

and just to make this even weirder, you can actually apply the resource with rules, but when deleted from state and reimported its missing the rules

ami-descope commented 11 months ago

@mikhailshilkov any update on this?

ami-descope commented 10 months ago

@mikhailshilkov there is no milestone set or issue assigned to someone, can we please get an update on this?

zbuchheit commented 7 months ago

when I attempt this now on the latest version I get panic: fatal: An assertion has failed: Missing key: responses when attempting to import

Full Panic:

Previewing import (dev)

View Live: redacted

  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev::cloudflare-ts::pulumi:pulumi:Stack::cloudflare-ts-dev]
    rulesetId: "redacted"
    zoneId   : "redacted"
panic: fatal: An assertion has failed: Missing key: responses

goroutine 55 [running]:

github.com/pulumi/pulumi/sdk/v3/go/common/util/contract.failfast(...)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.111.1/go/common/util/contract/failfast.go:23

github.com/pulumi/pulumi/sdk/v3/go/common/util/contract.Assertf(0xb0?, {0x10151b36c?, 0x9?}, {0x14000e54aa8?, 0x11?, 0x140006e5fa8?})

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.111.1/go/common/util/contract/assert.go:35 +0xe0

github.com/pulumi/pulumi-terraform-bridge/pf/internal/schemashim.(*objectPseudoResource).Get(0x14000c19b30?, {0x140006e5fb0, 0x9})

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/pf@v0.31.0/internal/schemashim/object_pseudoresource.go:110 +0x70

github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.getSchema(...)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.78.0/pkg/tfbridge/schema.go:266

github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.getInfoFromPulumiName({0x14000c19b30, 0x9}, {0x101c3e6a8, 0x1400017f130}, 0x10151f3c0?, 0x0)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.78.0/pkg/tfbridge/schema.go:1424 +0x184

github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.extractSchemaInputs({{0x101b77c80?, 0x1400116c510?}}, {0x101c4df58, 0x1400065bdc0}, 0x14000720700?, 0x0)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.78.0/pkg/tfbridge/schema.go:1681 +0x21c

github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.extractSchemaInputs({{0x101b77c80?, 0x1400116bec0?}}, {0x101c4de98, 0x14000beacc0}, 0x14000720620?, 0x0)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.78.0/pkg/tfbridge/schema.go:1687 +0x2bc

github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.extractSchemaInputs({{0x1018f0cc0?, 0x140001142d0?}}, {0x101c4df58, 0x1400065b620}, 0x14000d6bf70?, 0x0)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.78.0/pkg/tfbridge/schema.go:1662 +0x5d4

github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.extractSchemaInputs({{0x101b77c80?, 0x1400116bd40?}}, {0x101c4de98, 0x14000beac00}, 0x14000e551d0?, 0x0)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.78.0/pkg/tfbridge/schema.go:1687 +0x2bc

github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.ExtractInputsFromOutputs(0x101c41830?, 0x14000d65d10?, {0x101c3e1e0?, 0x14000b951a0}, 0x140006aafc0, 0x0)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.78.0/pkg/tfbridge/schema.go:1718 +0x134

github.com/pulumi/pulumi-terraform-bridge/pf/tfbridge.(*provider).ReadWithContext(0x14000732380, {0x101c39718?, 0x140005fe6c0?}, {0x140006080a0, 0x49}, {0x14000608050, 0x46}, 0x100921e58?, 0x140005fe720)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/pf@v0.31.0/tfbridge/provider_read.go:84 +0x2c8

github.com/pulumi/pulumi-terraform-bridge/pf/internal/plugin.(*providerServer).Read(0x14000d70f90, {0x101c39718, 0x140005fe6c0}, 0x14000f8f620)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/pf@v0.31.0/internal/plugin/provider_server.go:380 +0x1a0

github.com/pulumi/pulumi-terraform-bridge/x/muxer.(*muxer).Read.func1({0x101c4d678?, 0x14000d70f90?})

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.8/muxer.go:381 +0x3c

github.com/pulumi/pulumi-terraform-bridge/x/muxer.resourceMethod[...](0x14000c80780?, 0x14000ae76c8, 0x14000ae76a8?)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.8/muxer.go:334 +0xc4

github.com/pulumi/pulumi-terraform-bridge/x/muxer.(*muxer).Read(0x14000ae7708?, {0x101c39718?, 0x140005fe6c0?}, 0x10193dec0?)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.8/muxer.go:380 +0x5c

github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Read_Handler.func1({0x101c39718, 0x140005fe6c0}, {0x101b2a4a0?, 0x14000f8f620})

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.111.1/proto/go/provider_grpc.pb.go:611 +0x74

github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc.OpenTracingServerInterceptor.func1({0x101c39718, 0x140004c3860}, {0x101b2a4a0, 0x14000f8f620}, 0x14000e8c940, 0x140009ded38)

    /home/runner/go/pkg/mod/github.com/grpc-ecosystem/grpc-opentracing@v0.0.0-20180507213350-8e809c8a8645/go/otgrpc/server.go:57 +0x2e8

github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Read_Handler({0x101bdc480?, 0x14000c80780}, {0x101c39718, 0x140004c3860}, 0x140006b1000, 0x14000af61a0)

    /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.111.1/proto/go/provider_grpc.pb.go:613 +0x12c

google.golang.org/grpc.(*Server).processUnaryRPC(0x140005b2800, {0x101c39718, 0x140004c37a0}, {0x101c44160, 0x14000b18d00}, 0x140006f0c60, 0x14000d71050, 0x1028ed118, 0x0)

    /home/runner/go/pkg/mod/google.golang.org/grpc@v1.62.0/server.go:1383 +0xb8c

google.golang.org/grpc.(*Server).handleStream(0x140005b2800, {0x101c44160, 0x14000b18d00}, 0x140006f0c60)

    /home/runner/go/pkg/mod/google.golang.org/grpc@v1.62.0/server.go:1794 +0xc70

google.golang.org/grpc.(*Server).serveStreams.func2.1()

    /home/runner/go/pkg/mod/google.golang.org/grpc@v1.62.0/server.go:1027 +0x8c

created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 49

    /home/runner/go/pkg/mod/google.golang.org/grpc@v1.62.0/server.go:1038 +0x150

Resources:
    2 unchanged
zbuchheit commented 7 months ago

interestingly, when I run it with --diff I get a different panic panic: fatal: An assertion has failed: Missing key: autominifies

t0yv0 commented 7 months ago

CC @iwahbe - the PF implementation was calling into tfbridge.ExtractInputsFromOutputs for import to avoid implementing this functionality natively.

https://github.com/pulumi/pulumi-terraform-bridge/blob/master/pf/tfbridge/provider_read.go#L84 - I recall having some suspicions around this, but don't have a ready explanation for the panic here.

mikhailshilkov commented 7 months ago

I think the original problem was partially fixed by https://github.com/pulumi/pulumi-terraform-bridge/pull/1659 (that got pulled in 5.20.0 of the provider), but now it replaced the original issue with a panic that we need to solve.

mikhailshilkov commented 7 months ago

An assertion has failed: Missing key: responses

The original field is called response here

Missing key: autominifies

The original field is called autominify here

It looks like we are butchering plural-singular translation.

iwahbe commented 7 months ago

I have successfully reproduced the panic with this pulumi program and pulumi-cloudflare v5.24.0:

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

const zone = cloudflare.getZoneOutput({
  accountId: new pulumi.Config().require("cloudflare-account-id"),
  name: "pulumi-cloudflare-demo.com",
})

const ruleset = new cloudflare.Ruleset("domain-rate-limit", {
  name: "domain-rate-limit",
  kind: "zone",
  zoneId: zone.zoneId,
  phase: "http_request_transform",

  rules: [{
    action: "rewrite",
    actionParameters: {
      uri: {
        path: {
          value: "/my-new-route"
        }
      }
    },

    expression: "(http.host eq \"example.com\" and http.request.uri.path eq \"/old-path\")",
    description: "example URI path transform rule",
    enabled: true,
  }],
});

export const importName = pulumi.interpolate`zone/${zone.id}/${ruleset.id}`;

cloudflare.Ruleset.get("imported", importName);

For Pulumi employees, the keys necessary to repro are in providers.all (ESC).

iwahbe commented 7 months ago

Keeping open until the bridge release that contains the fix is released as part of pulumi-cloudflare.