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

Server routes order #10

Closed
fmalykh opened this issue Nov 2, 2021 · 20 comments · Fixed by #11
Closed

Server routes order #10

fmalykh opened this issue Nov 2, 2021 · 20 comments · Fixed by #11

Comments

@fmalykh
Copy link

fmalykh commented Nov 2, 2021

Hi,
Thanks for the great provider, I've been dreaming to have it for such a long time.
After importing existing server as pritunl_server resource, terraform_plan suggests multiple changes to the routes which looks like something related to routes ordering:

resource "pritunl_server" "my-vpn" {
    name    = "my-vpn"
    port    = 1196
    network = "10.8.35.0/25"
  
    organization_ids = [
      pritunl_organization.this_orgs["_tools_"].id
    ]

    route {
      network = "10.1.0.0/16"
      comment = "route-1"
      nat     = true
    }
    route {
      network = "10.2.4.234/32"
      comment = "route-2"
      nat     = true
    }  
    route {
      network = "10.4.0.0/16"
      comment = "route-3"
      nat     = true
    }
  }

$terraform plan --target pritunl_server.my-vpn
pritunl_server.my-vpn: Refreshing state... [id=5e45018371f6b2996983beac]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # pritunl_server.my-vpn will be updated in-place
  ~ resource "pritunl_server" "my-vpn" {
        id                 = "5e45018371f6b2996983beac"
        name               = "my-vpn"
        # (31 unchanged attributes hidden)

      ~ route {
          ~ comment = "route-2" -> "route-1"
          ~ network = "10.2.4.234/32" -> "10.1.0.0/16"
            # (1 unchanged attribute hidden)
        }
      ~ route {
          ~ comment = "route-3" -> "route-2"
          ~ network = "10.4.0.0/16" -> "10.2.4.234/32"
            # (1 unchanged attribute hidden)
        }
      ~ route {
          ~ comment = "route-1" -> "route-3"
          ~ network = "10.1.0.0/16" -> "10.4.0.0/16"
            # (1 unchanged attribute hidden)
        }
    }

Pritunl API returns data in the same order as stated in pritunl_server resource :
'GET', '/server/5e45018371f6b2996983beac/route'

[
   {
      "comment":"None",
      "nat_netmap":"None",
      "network":"10.8.35.0/25",
      "vpc_region":"None",
      "net_gateway":false,
      "network_link":false,
      "metric":"None",
      "server":"5e45018371f6b2996983beac",
      "nat":false,
      "virtual_network":true,
      "vpc_id":"None",
      "advertise":"None",
      "link_virtual_network":false,
      "nat_interface":"None",
      "id":"31302e382e33352e302f3235",
      "server_link":false
   },
   {
      "comment":"route-1",
      "nat_netmap":"None",
      "network":"10.1.0.0/16",
      "vpc_region":"None",
      "net_gateway":false,
      "network_link":false,
      "metric":"None",
      "server":"5e45018371f6b2996983beac",
      "nat":true,
      "virtual_network":false,
      "vpc_id":"None",
      "advertise":false,
      "link_virtual_network":false,
      "nat_interface":"None",
      "id":"31302e312e302e302f3136",
      "server_link":false
   },
   {
      "comment":"route-2",
      "nat_netmap":"None",
      "network":"10.2.4.234/32",
      "vpc_region":"None",
      "net_gateway":false,
      "network_link":false,
      "metric":"None",
      "server":"5e45018371f6b2996983beac",
      "nat":true,
      "virtual_network":false,
      "vpc_id":"None",
      "advertise":false,
      "link_virtual_network":false,
      "nat_interface":"None",
      "id":"31302e322e342e3233342f3332",
      "server_link":false
   },
   {
      "comment":"route-3",
      "nat_netmap":"None",
      "network":"10.4.0.0/16",
      "vpc_region":"None",
      "net_gateway":false,
      "network_link":false,
      "metric":"None",
      "server":"5e45018371f6b2996983beac",
      "nat":true,
      "virtual_network":false,
      "vpc_id":"None",
      "advertise":false,
      "link_virtual_network":false,
      "nat_interface":"None",
      "id":"31302e342e302e302f3136",
      "server_link":false
   }
]

Is there anything to do with this? Thanks!

@dlethin
Copy link

dlethin commented Nov 2, 2021

I had run into this as well. Thanks for reporting it. I would guess the underlying issue is that the type for routes is defined as a TypeList where is probably needs to be TypeSet, as order doesn't probably matter routing entries for a server.
I would think this is probably going to be the same thing for organizations as well

@disc
Copy link
Owner

disc commented Nov 2, 2021

Hey @fmalykh What's version of the provider do you use?

I fixed a few version ago in this commit https://github.com/disc/terraform-provider-pritunl/blob/master/internal/provider/resource_server.go#L999
Now the order in the tf-file uses when store save into a file. The issue might reproduce once after first import. Do you face the problem more than once - with any terraform apply command?

The original issue is hashicorp/terraform-plugin-sdk#477

@fmalykh
Copy link
Author

fmalykh commented Nov 2, 2021

Hey @disc
I'm using v0.1.1

I've just imported server resources and didn't apply yet - the change looked scary for me as I thought routes may get in conflict during the update process.

@disc
Copy link
Owner

disc commented Nov 2, 2021

There are two options:

  • update routes in your tf-file in the same way as in a state
  • or just execute terraform apply and the state will be updated

@disc
Copy link
Owner

disc commented Nov 2, 2021

By the way I'm going to check why the state has an incorrect order for routes after import

@fmalykh
Copy link
Author

fmalykh commented Nov 2, 2021

Just to let you know - provider successfully renamed routes / networks and now config is in sync with state. Running plan again doesn't suggest any changes. Lovely!

@disc
Copy link
Owner

disc commented Nov 2, 2021

I've tried to reproduce your situation but the state file contains routes in the proper way and terraform apply doesn't show any updates in the routes.

my terraform.tfstate file after importing the server
terraform import pritunl_server.test xxx

  "version": 4,
  "terraform_version": "1.0.3",
  "serial": 1,
  "lineage": "be257093-7e67-42ce-2bdf-097af78dc231",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "pritunl_server",
      "name": "test",
      "provider": "provider[\"registry.terraform.io/disc/pritunl\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            // ... removed unnecessary state values
            "route": [
              {
                "comment": "route-1",
                "nat": true,
                "network": "10.1.0.0/16"
              },
              {
                "comment": "route-2",
                "nat": true,
                "network": "10.2.4.234/32"
              },
              {
                "comment": "route-3",
                "nat": true,
                "network": "10.4.0.0/16"
              }
            ],
            // ... removed unnecessary state values
          },
          "sensitive_attributes": [],
          "private": "xxx"
        }
      ]
    }
  ]
}

Can you import your server one more time and check the routes order in the "route" object of the state file?

@dlethin
Copy link

dlethin commented Nov 2, 2021

FWIW, I run into the ordering of routes intermittently running the test make target:

❯ make test
Error: No such container: tf_pritunl_acc_test
[REDACTED]
sleep 10
./tools/wait-for-it.sh localhost:27017 -- echo "mongodb is up"
wait-for-it.sh: waiting 15 seconds for localhost:27017
wait-for-it.sh: localhost:27017 is available after 0 seconds
mongodb is up
# enables an api access for the pritunl user, updates an api token and secret
MongoDB shell version: 3.2.22
connecting to: test
switched to db pritunl
WriteResult({ "nMatched" : 1, "nUpserted" : 0, "nModified" : 1 })
bye
TF_ACC=1 \
	PRITUNL_URL="https://localhost/" \
	PRITUNL_INSECURE="true" \
	PRITUNL_TOKEN=tfacctest_token \
	PRITUNL_SECRET=tfacctest_secret \
	go test -v -cover -count 1 ./internal/provider
=== RUN   TestDataSourceHost
=== PAUSE TestDataSourceHost
=== RUN   TestAccOrganization_basic
=== PAUSE TestAccOrganization_basic
=== RUN   TestGetServer_basic
--- PASS: TestGetServer_basic (50.69s)
=== RUN   TestGetServer_with_attached_organization
--- PASS: TestGetServer_with_attached_organization (33.14s)
=== RUN   TestGetServer_with_a_few_attached_organizations
--- PASS: TestGetServer_with_a_few_attached_organizations (33.44s)
=== RUN   TestGetServer_with_attached_route
    resource_server_test.go:166: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

        (map[string]string) (len=1) {
         (string) (len=6) "status": (string) (len=7) "offline"
        }


        (map[string]string) (len=1) {
         (string) (len=6) "status": (string) (len=7) "pending"
        }
--- FAIL: TestGetServer_with_attached_route (31.62s)
=== RUN   TestGetServer_with_a_few_attached_routes
    resource_server_test.go:216: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

        (map[string]string) (len=2) {
         (string) (len=15) "route.0.network": (string) (len=11) "10.3.0.0/24",
         (string) (len=15) "route.1.network": (string) (len=11) "10.2.0.0/24"
        }


        (map[string]string) (len=2) {
         (string) (len=15) "route.0.network": (string) (len=11) "10.2.0.0/24",
         (string) (len=15) "route.1.network": (string) (len=11) "10.3.0.0/24"
        }
--- FAIL: TestGetServer_with_a_few_attached_routes (25.03s)
=== RUN   TestGetServer_with_invalid_route
--- PASS: TestGetServer_with_invalid_route (7.08s)
=== RUN   TestCreateServer_with_invalid_network
--- PASS: TestCreateServer_with_invalid_network (9.41s)
=== RUN   TestCreateServer_with_unsupported_network
--- PASS: TestCreateServer_with_unsupported_network (21.67s)
=== RUN   TestCreateServer_with_invalid_bind_address
--- PASS: TestCreateServer_with_invalid_bind_address (20.93s)
=== RUN   TestGetServer_with_default_host
--- PASS: TestGetServer_with_default_host (19.15s)
=== RUN   TestGetServer_without_hosts
--- PASS: TestGetServer_without_hosts (32.28s)
=== RUN   TestAccUser_basic
=== PAUSE TestAccUser_basic
=== CONT  TestDataSourceHost
=== CONT  TestAccUser_basic
=== CONT  TestAccOrganization_basic
--- PASS: TestDataSourceHost (24.42s)
--- PASS: TestAccOrganization_basic (54.67s)
--- PASS: TestAccUser_basic (56.05s)
FAIL
coverage: 64.3% of statements
FAIL	github.com/disc/terraform-provider-pritunl/internal/provider	340.730s
FAIL
make: *** [test] Error 1

@fmalykh
Copy link
Author

fmalykh commented Nov 2, 2021

I've imported 7 servers and all of them showed that routes require reordering.
Can this be related to Pritunl version? I'm running pretty old one v1.29.2276.91 bb831e

@disc
Copy link
Owner

disc commented Nov 2, 2021

No, I don't think so.
Could you share the part of tour state-file with routes list?

@fmalykh
Copy link
Author

fmalykh commented Nov 2, 2021

Sure, here it is:

"route": [
    {
      "comment": "route-1",
      "nat": true,
      "network": "10.1.0.0/16"
    },
    {
      "comment": "route-2",
      "nat": true,
      "network": "10.2.4.234/32"
    },
    {
      "comment": "route-3",
      "nat": true,
      "network": "10.4.0.0/16"
    }
  ],
  "search_domain": "/redacted/",
  "status": "online",
  "vxlan": false
}

@disc
Copy link
Owner

disc commented Nov 2, 2021

State looks properly sorted, if you have the same order in your tf-file it should be re-ordered

@disc
Copy link
Owner

disc commented Nov 2, 2021

Oh, you know, it seems I've reprouced the issue. Have no steps yet, but after 5-6 imports of the same server I've got a wrong order in the state file:

"route": [
              {
                "comment": "route-2",
                "nat": true,
                "network": "10.2.4.234/32"
              },
              {
                "comment": "route-3",
                "nat": true,
                "network": "10.4.0.0/16"
              },
              {
                "comment": "route-1",
                "nat": true,
                "network": "10.1.0.0/16"
              }
            ],

I will look into it

disc added a commit that referenced this issue Nov 2, 2021
…ting step because there is no previous (or setup in advance) state
@disc
Copy link
Owner

disc commented Nov 2, 2021

I've found the issue and PR is ready.
There was no "declared" routes, groups or any other attributes from a tf-file during importing process.
In this case will be used an order returned from the Pritunl API

@disc disc closed this as completed in #11 Nov 2, 2021
@disc
Copy link
Owner

disc commented Nov 2, 2021

It should be fixed in next release v.0.1.2

@fmalykh
Copy link
Author

fmalykh commented Dec 6, 2021

Hi @disc 🙏
I feel like I hit something similar again:

Here is a server config (shortened):

resource "pritunl_server" "my_server" {
  name            = "my_server"

  route {
    comment = "route_1"
    nat     = true
    network = "10.1.0.0/16"
  }
  route {
    comment = "route_2"
    nat     = true
    network = "10.2.0.0/16"
  }
  route {
    comment = "route_3"
    nat     = true
    network = "10.3.0.0/16"
  }
  route {
    comment = "route_4"
    nat     = true
    network = "10.4.0.0/16"
  }
  route {
    comment = "route_5"
    nat     = true
    network = "10.6.11.0/24"
  }
  route {
    comment = "route_5"
    nat     = true
    network = "10.6.12.0/24"
  }
  route {
    comment = "route_7"
    nat     = true
    network = "10.6.15.0/24"
  }

}

Here is its state:

-> terraform state show pritunl_server.my_server
# pritunl_server.my_server:
resource "pritunl_server" "my_server" {
    route {
        comment = "route_1"
        nat     = true
        network = "10.1.0.0/16"
    }
    route {
        comment = "route_2"
        nat     = true
        network = "10.2.0.0/16"
    }
    route {
        comment = "route_3"
        nat     = true
        network = "10.3.0.0/16"
    }
    route {
        comment = "route_4"
        nat     = true
        network = "10.4.0.0/16"
    }
    route {
        comment = "route_5"
        nat     = true
        network = "10.6.11.0/24"
    }
    route {
        comment = "route_5"
        nat     = true
        network = "10.6.12.0/24"
    }
    route {
        comment = "route_7"
        nat     = true
        network = "10.6.15.0/24"
    }
}

Terraform plan shows nothing to do.

Then I delete one of the routes so that .tf config looks the following:

resource "pritunl_server" "my_server" {
  name            = "my_server"

  route {
    comment = "route_1"
    nat     = true
    network = "10.1.0.0/16"
  }
  route {
    comment = "route_2"
    nat     = true
    network = "10.2.0.0/16"
  }
  route {
    comment = "route_4"
    nat     = true
    network = "10.4.0.0/16"
  }
  route {
    comment = "route_5"
    nat     = true
    network = "10.6.11.0/24"
  }
  route {
    comment = "route_5"
    nat     = true
    network = "10.6.12.0/24"
  }
  route {
    comment = "route_7"
    nat     = true
    network = "10.6.15.0/24"
  }

}

And the plan shows something I can't explain:

terraform plan --target pritunl_server.my_server

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # pritunl_server.my_server will be updated in-place
  ~ resource "pritunl_server" "my_server" {
        id                 = "5e4bf83c71f6b2996987cca8"
        name               = "my_server"
        # (31 unchanged attributes hidden)

      ~ route {
          ~ comment = "route_3" -> "route_4"
          ~ network = "10.3.0.0/16" -> "10.4.0.0/16"
            # (1 unchanged attribute hidden)
        }
      ~ route {
          ~ comment = "route_4" -> "route_5"
          ~ network = "10.4.0.0/16" -> "10.6.11.0/24"
            # (1 unchanged attribute hidden)
        }
      ~ route {
          ~ network = "10.6.11.0/24" -> "10.6.12.0/24"
            # (2 unchanged attributes hidden)
        }
      ~ route {
          ~ comment = "route_5" -> "route_7"
          ~ network = "10.6.12.0/24" -> "10.6.15.0/24"
            # (1 unchanged attribute hidden)
        }
      - route {
          - comment = "route_7" -> null
          - nat     = true -> null
          - network = "10.6.15.0/24" -> null
        }
        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Warning: Resource targeting is in effect

@disc
Copy link
Owner

disc commented Dec 8, 2021

@fmalykh Did it happen after import and just once?
I've seen such a behavior right after the server's import and after first terraform apply routes' order was correct.
Have you got the same situation?

@fmalykh
Copy link
Author

fmalykh commented Dec 10, 2021

@disc hey,
That's the first time I tried to make a change in the routes through terraform.
Before deleting a route terraform plan showed nothing to change. Then I removed a route from tf and plan looks weird.
Thanks!

@disc
Copy link
Owner

disc commented Dec 10, 2021

@fmalykh maybe you meant the plan looked weird, it might be because routes were re-sorted based on one deleted route, for example. It shows just in-place replacements without deleting other routes?

@fmalykh
Copy link
Author

fmalykh commented Dec 11, 2021

Yeah, it shows in-place replacements and doesn't mention deleting the route I removed from pritunl_server resource

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants