fix: ALL implicit privileges equality check

[talbx]

When using resources postgresql_default_privileges or postgresql_grant you have the option to define a fine grained set of privileges (for example only CONNECT for object_type database) or all at once, either by setting all privileges (for database: ["CREATE", "CONNECT", "TEMPORARY"] or simply ["ALL"].

The latter one is quite handy, since you do not have to find out which privileges are part of it. When using ALL, the provider will grant the privileges by using the set of the actual grants, so in this scenario by granting "CREATE", "CONNECT", "TEMPORARY" and not ALL. Probably because ALL is not known to postgres. 🤷🏼‍♂️

Anyways, when executing a terraform plan the next time after the apply for ALL, the diff will find out, that not ALL is set as privilege, but "CREATE", "CONNECT", "TEMPORARY". Since these privileges are the same or have the same semantic to it, there shouldn't be really a update each time you use terraform, because the state on the DB is totally fine and according to the resource.

To prevent this unnecessary modification, I implemented some more detailed equality check for all object_types except column.

I tested the changes locally with a dockerized postgres and also wrote some unit tests for it.

Would love to hear some feedback about it, especially since it's my first time working on a terraform provider..

Cheers

[talbx]

@cyrilgdn any chance you can take a look at that?

[talbx]

Thanks for opening this PR and many apologies for the response delay, I didn't have the time to work on this provider.

no worries, thanks for checking in again :)

Thanks for the unittest on the new function 👍 It would be nice to also have acceptance test about that on postgresql_grant resource, there's already many tests in postgresql_grant_test.go You can have a look at this one for example: https://github.com/cyrilgdn/terraform-provider-postgresql/blob/master/postgresql/resource_postgresql_grant_test.go#L1082-L1140

I will give it a shot.

[talbx]

might relate to #303

[dzavalkin-scayle]

Any chance this PR will be merged soon @cyrilgdn @talbx 🙏 ? We are tired seeing perpetual diff in all places where we use postgresql_grant resource...

[talbx]

Any chance this PR will be merged soon @cyrilgdn @talbx 🙏 ? We are tired seeing perpetual diff in all places where we use postgresql_grant resource...

I am still waiting for further feedback by @cyrilgdn; although i am not sure it will resolve your problems, @dzavalkin-scayle since you haven't specified them in detail and also, this PR solely focusses on the implicit "ALL" diff. I haven't tested if this PR fixes other cases aswell.

[dzavalkin-scayle]

i am not sure it will resolve your problems, @dzavalkin-scayle since you haven't specified them in detail and also, this PR solely focusses on the implicit "ALL" diff. I haven't tested, if this PR fixes other cases aswell.

Hmm I was under impression that it will fix diff issue entirely, no matter what are the privileges... Without this fix we need to add privileges to ignore_changes which isn't nice at all and raises a question if we should look for another postgresql provider then 😞

[talbx]

Hmm I was under impression that it will fix diff issue entirely, no matter what are the privileges... Without this fix we need to add privileges to ignore_changes which isn't nice at all and raises a question if we should look for another postgresql provider then 😞

@dzavalkin-scayle can you give me a Terraform snippet to reproduce your problem? i can recheck if its included

[dzavalkin-scayle]

@talbx Thanks a lot! Here is the code we have:

locals {
  default_privileges = merge([
    for owner, users in var.rds_postgres_user_default_privileges :
    {
      for user in users : "${owner}-${user}" => {
        owner = owner
        user  = user
      }
    }
  ]...)
}

resource "random_password" "user_password" {
  for_each = { for k, v in var.rds_postgres_users : k => v if !lookup(v, "allow_iam_role_access", false) }

  length = 42

  special = true
  numeric = true
  lower   = true
  upper   = true

  override_special = "!()-_=[]{}"
  min_numeric      = 3
  min_lower        = 3
  min_upper        = 3
}

resource "postgresql_role" "postgres_users" {
  for_each = var.rds_postgres_users

  name     = each.key
  login    = true
  password = lookup(each.value, "allow_iam_role_access", false) ? "" : random_password.user_password[each.key].result
  roles    = lookup(each.value, "allow_iam_role_access", false) ? ["rds_iam"] : []
}

resource "postgresql_default_privileges" "postgresql_user_default_table_privileges" {
  for_each = {
    for key, value in local.default_privileges :
    key => value
    if lookup(var.rds_postgres_users[value.user], "table_privileges", "") != ""
  }

  database    = var.rds_database_name
  object_type = "table"
  owner       = each.value.owner
  privileges  = split(",", var.rds_postgres_users[each.value.user].table_privileges)
  role        = each.value.user
  schema      = "public"

  depends_on = [
    postgresql_role.postgres_users
  ]
}

resource "postgresql_default_privileges" "postgresql_user_default_sequence_privileges" {
  for_each = {
    for key, value in local.default_privileges :
    key => value
    if lookup(var.rds_postgres_users[value.user], "sequence_privileges", "") != ""
  }

  database    = var.rds_database_name
  object_type = "sequence"
  owner       = each.value.owner
  privileges  = split(",", var.rds_postgres_users[each.value.user].sequence_privileges)
  role        = each.value.user
  schema      = "public"

  depends_on = [
    postgresql_role.postgres_users
  ]
}

resource "postgresql_default_privileges" "postgresql_user_default_routine_privileges" {
  for_each = {
    for key, value in local.default_privileges :
    key => value
    if lookup(var.rds_postgres_users[value.user], "routine_privileges", "") != ""
  }

  database    = var.rds_database_name
  object_type = "function"
  owner       = each.value.owner
  privileges  = split(",", var.rds_postgres_users[each.value.user].routine_privileges)
  role        = each.value.user
  schema      = "public"

  depends_on = [
    postgresql_role.postgres_users
  ]
}

resource "postgresql_grant" "postgresql_user_table_privileges" {
  for_each = {
    for username, user_settings in var.rds_postgres_users :
    username => user_settings
    if lookup(user_settings, "table_privileges", "") != ""
  }

  database          = var.rds_database_name
  object_type       = "table"
  privileges        = split(",", each.value.table_privileges)
  role              = each.key
  schema            = "public"
  with_grant_option = lookup(each.value, "grant", false)
  depends_on = [
    postgresql_role.postgres_users
  ]
}

resource "postgresql_grant" "postgresql_user_database_privileges" {
  for_each = {
    for username, user_settings in var.rds_postgres_users :
    username => user_settings
    if lookup(user_settings, "database_privileges", "") != ""
  }

  database          = var.rds_database_name
  object_type       = "database"
  privileges        = split(",", each.value.database_privileges)
  role              = each.key
  with_grant_option = lookup(each.value, "grant", false)
  depends_on = [
    postgresql_role.postgres_users
  ]
}

resource "postgresql_grant" "postgresql_user_routine_privileges" {
  for_each = {
    for username, user_settings in var.rds_postgres_users :
    username => user_settings
    if lookup(user_settings, "routine_privileges", "") != ""
  }

  database          = var.rds_database_name
  role              = each.key
  object_type       = "routine"
  schema            = "public"
  privileges        = split(",", each.value["routine_privileges"])
  with_grant_option = lookup(each.value, "grant", false)
  depends_on = [
    postgresql_role.postgres_users
  ]
}

resource "postgresql_grant" "postgresql_user_schema_privileges" {
  for_each = {
    for username, user_settings in var.rds_postgres_users :
    username => user_settings
    if lookup(user_settings, "schema_privileges", "") != ""
  }

  database          = var.rds_database_name
  object_type       = "schema"
  privileges        = split(",", each.value["schema_privileges"])
  role              = each.key
  schema            = "public"
  with_grant_option = lookup(each.value, "grant", false)
  depends_on = [
    postgresql_role.postgres_users
  ]
}

resource "postgresql_grant" "postgresql_user_sequence_privileges" {
  for_each = {
    for username, user_settings in var.rds_postgres_users :
    username => user_settings
    if lookup(user_settings, "sequence_privileges", "") != ""
  }

  database          = var.rds_database_name
  role              = each.key
  object_type       = "sequence"
  schema            = "public"
  privileges        = split(",", each.value["sequence_privileges"])
  with_grant_option = lookup(each.value, "grant", false)
  depends_on = [
    postgresql_role.postgres_users
  ]
}

and values for input variables are (passed from Terragrunt):

  rds_postgres_users = {
    app_schema = {
      database_privileges = "CONNECT"
      grant               = true
      schema_privileges   = "CREATE,USAGE"
      routine_privileges  = "EXECUTE"
      sequence_privileges = "SELECT,UPDATE"
      table_privileges    = "SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES,TRIGGER"
    },
    app_unrestricted = {
      database_privileges = "CONNECT"
      routine_privileges  = "EXECUTE"
      schema_privileges   = "USAGE"
      sequence_privileges = "SELECT,UPDATE"
      table_privileges    = "SELECT,INSERT,UPDATE,DELETE,TRIGGER"
    },
    readonly_user = {
      allow_iam_role_access = true
      schema_privileges     = "USAGE"
      sequence_privileges   = "SELECT"
      table_privileges      = "SELECT"
    },
    rw_user = {
      allow_iam_role_access = true
      database_privileges   = "CONNECT"
      routine_privileges    = "EXECUTE"
      schema_privileges     = "USAGE"
      sequence_privileges   = "SELECT"
      table_privileges      = "SELECT,UPDATE,INSERT,DELETE,TRIGGER"
    }
  }
  rds_postgres_user_default_privileges = {
    app_schema = [
      "app_unrestricted",
      "readonly_user",
      "rw_user",
    ],
  }

Please let me know if you can use this code or you want me to simplify it somehow (though in this case we won't test our exact use cases).

[talbx]

@dzavalkin-scayle It would be nice if you could simplify that setup to a minimum so I can test it more easily. Would also appreciate it if you could make it self-contained so I can just copy paste and run. Thanks in advance!

[talbx]

@cyrilgdn can you take another look 👀

[igor-nikiforov]

@cyrilgdn just a friendly reminder about this very import PR.