Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

Allow grant to specific tables #156

Closed
wants to merge 15 commits into from
Closed

Allow grant to specific tables #156

wants to merge 15 commits into from

Conversation

jmks
Copy link

@jmks jmks commented Jul 20, 2020

Related to #85

I have the use-case of granting specific privileges to specific tables (and others seemed interested as well, so took a shot at doing that.

resource "postgresql_grant" "test" {
  role        = "some_role"
  object_type = "table"
  tables      = ["table1", "table2"]
  privileges  = ["SELECT"]
}

should produce the SQL GRANT SELECT ON TABLE table1,table2 TO "some_role".

I was interested in writing an acceptance test but could not get them running. I tried this:

➜ TF_ACC=1 TF_LOG=INFO go test -v ./postgresql -run ^TestAccPostgresqlGrant$
=== RUN   TestAccPostgresqlGrant
2020/07/19 18:43:54 [INFO] PostgreSQL DSN: `host=localhost port=5432 dbname=postgres user='' password=<redacted> sslmode='' connect_timeout=0`
    TestAccPostgresqlGrant: utils_test.go:77: could not execute query CREATE ROLE tf_tests_role_1595198634363977000 LOGIN ENCRYPTED PASSWORD 'testpwd': pq: SSL is not enabled on the server
--- FAIL: TestAccPostgresqlGrant (0.01s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-postgresql/postgresql	0.037s
FAIL

I could try harder 😄

@ghost ghost added the size/M label Jul 20, 2020
@cyrilgdn
Copy link
Contributor

@jmks Thanks a lot for working on this! It's indeed a high demanded feature.

I'll try to take, a look as soon as I can (can take a little while though).

Regarding the tests, if you running them against a Postgresql server started in a Docker container, you need to set the sslmode setting to disable (export PGSSLMODE=disable for example)

You can see here how it's configured for Travis and I use the same script manually with the docker-compose.yml to start Postgres:

cd tests/
docker-compose up -d
source switch_superuser.sh
# run the tests

@jmks
Copy link
Author

jmks commented Jul 20, 2020

@cyrilgdn Ahh. The README mentioned a tests/env.sh but I see its contents were moved around now. Thanks!

@cyrilgdn
Copy link
Contributor

@cyrilgdn Ahh. The README mentioned a tests/env.sh but I see its contents were moved around now. Thanks!

Yes, recently but indeed I forgot to update the README 🤦 , I'll fix it.

@jmks
Copy link
Author

jmks commented Jul 20, 2020

OK, I got the acceptance tests running now.

I'm realizing pulling the tables / table-level privileges information from the database is trickier than I thought.
I may need a couple days to go through that.

@ghost ghost added size/XL and removed size/M labels Aug 7, 2020
@jmks
Copy link
Author

jmks commented Aug 7, 2020

Hey @cyrilgdn

I finally got an Acceptance test working. It turns out when they fail, it may indicate something is wrong 😄

A couple design issues that came up:

  1. To read the privileges for particular tables, I needed a lot of metadata from the resource Id.

    • I've observed other providers jamming a bunch of structured data in the Id, so I went with that approach.
  2. A lot of changes to this resource for tables.

    • This resource seems pretty complex and I made it worse 😆
    • I don't know when the resource is too complex to be broken up as you suggested here

It seems to do what I expect locally (revoke all then grant, in all cases), so think it can be reviewed now.

@jmks jmks closed this by deleting the head repository May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants