feat: Add postgresql_security_label resource

[stanleyz]

This PR is based on the great job @jbunting did in #365, which fixed the quoted identifier for object name and provider in the pg_seclabels table.

Additional changes:

1. Update the doc to explain the import process

[stanleyz]

@cyrilgdn can you please take a look? Thanks.

[cyrilgdn]

Hi @stanleyz ,

Can't you push in @jbunting PR directly? (it's possible if the PR author allowed it)

Update main.go to add -debug option

Could you create a specific PR for that so we don't mix unrelated stuff 🙏 ?

[stanleyz]

Can't you push in @jbunting PR directly? (it's possible if the PR author allowed it)

I tried to get the communication going in the original PR #365 but didn't see @jbunting responding, hence this PR.

Saying that however, this PR is based on and cherry-picked from the work @jbunting did in #365, so we are co-authored for this PR effectively.

Could you create a specific PR for that so we don't mix unrelated stuff 🙏 ?

Cool, I have removed that change from this PR, please take a look. Thanks.

[jbunting]

Sorry I haven't really had time to get back to this. I'm happy to change my pr to let you edit it (how?), or just close it out, whatever is easier.

[stanleyz]

Sorry I haven't really had time to get back to this. I'm happy to change my pr to let you edit it (how?), or just close it out, whatever is easier.

I'd suggest we close the previous one #365 and just use this one since the work is mostly done already.

[stanleyz]

@cyrilgdn can you please make it to the main if no issues found? Thanks.

The branch quickly turns out of data, I clicked on rebase which requires the checks to run again.

[cyrilgdn]

@cyrilgdn can you please make it to the main if no issues found? Thanks.

The branch quickly turns out of data, I clicked on rebase which requires the checks to run again.

I started the review yesterday but it takes time and I have a job too 😅 , so please be patient 🙏

[martindz]

thank you, guys, for your work and commitment.

[stanleyz]

@cyrilgdn can you please make it to the main if no issues found? Thanks.
The branch quickly turns out of data, I clicked on rebase which requires the checks to run again.

I started the review yesterday but it takes time and I have a job too 😅 , so please be patient 🙏

Even as powerful as @cyrilgdn, you still need a job, I don't believe it, must be a lie 😅

[cyrilgdn]

Thanks for your work 💪 , some minor comments/questions but otherwise 👍

[cyrilgdn]

Is there a point to scan objectType and provider as you use them in the filters of the request?

[stanleyz]

This is Read function right, I believe the point of the Read function in Terraform providers is to re-conciliate the state values with the actual value in the remote system, so yes, it's good to read and update the local.

I didn't include the objname in the scan because of the difficulties involved with handling the quote_ident, I probably should still do that, will push new commit to change this.

[stanleyz]

Thanks @cyrilgdn for the review, have updated with the below, please take a look, thanks.

1. Revert the if err := resourcePostgreSQLSecurityLabelUpdateImpl(db, d, pq.QuoteLiteral(label)); err != nil { in the resourcePostgreSQLSecurityLabelCreate function
2. Add block to resourcePostgreSQLSecurityLabelReadImpl to check whether objname and provider are in sync with the remote system.

[cyrilgdn]

The failing tests are unrelated to your PR, I'll have a look

actually it's related, cf #482 (comment)

[cyrilgdn]

Don't bother with the failing test on version 11, I was planning to remove this version from the tests as it's not supported by Postgres anymore.

[stanleyz]

Last question, why not quoteIdentifier everything to simplify (and avoid the need of this function)?

Adding quote to identifiers that doesn't have special characters is a no-op operation, PostgreSQL will remove the quote before persistence, for example "test_role" is saved as test_role, but "test-role" is saved as "test-role"

[cyrilgdn]

Last question, why not quoteIdentifier everything to simplify (and avoid the need of this function)?

Adding quote to identifiers that doesn't have special characters is a no-op operation, PostgreSQL will remove the quote before persistence, for example "test_role" is saved as test_role, but "test-role" is saved as "test-role"

Thanks, it's clear 👍 Weird behavior from Postgres side though as it does not behave similarly for other resources but it's cool you handle it properly!

[cyrilgdn]

Thanks a lot for your work @jbunting and @stanleyz 👏 💪

It will be release in the next days.

[stanleyz]

Thanks a lot for your work @jbunting and @stanleyz 👏 💪

It will be release in the next days.

Great, thanks @cyrilgdn .

[jarpoole]

@cyrilgdn any chance you could trigger a release when you have a moment?

[binnykmathew-tlnr]

@cyrilgdn - Nice, if you could make a release of this change, so I can avoid forking it out just to be able to use in my project :)

[martindz]

Seems it didn't get to last release on 22 Oct. Is there any plan for next one, any eta? Thanks

[cyrilgdn]

@stanleyz @jarpoole @binnykmathew-tlnr @martindz This has just been release in https://github.com/cyrilgdn/terraform-provider-postgresql/releases/tag/v1.25.0