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

Support for Cockroach DB #372

Open
kylepl opened this issue Nov 7, 2023 · 6 comments
Open

Support for Cockroach DB #372

kylepl opened this issue Nov 7, 2023 · 6 comments

Comments

@kylepl
Copy link

kylepl commented Nov 7, 2023

Hi,

I'm attempting to use this provider with Cockroach DB, which aims to be ~Postgres compatible. A few different bugs show that features relied upon by cockroach DB:

I've managed to hack-up a version such that I can at least support the Role resource, and tested it locally (commit). This just a proof-of-concept, but solves my immediate issue.

This provider already has conditional features depending on the version, so my question is, is it reasonable to expand this to support other "near-Postgres" databases? So Cockroach is one, Yugabyte another.

Options I can picture:

  • This is beyond the scope of this Provider, and this will target vanilla Postgres - thus a fork would be reasonable.
  • Extend the conditional features to not just be predicated on the Postgres version, but also on the type of postgres implementation (vanilla vs. cockroach vs. yugabyte vs...).

To add a few notes about how the conditional features could look:

  • The provider right now either deduces the version from SELECT version();, which does not work for Cockroach at least, but users can set expectedVersion. Ideally, parsing for Cockroach and other versions would be supported.
  • The provider would also take a parameter of which implementation it was using - i.e. vanilla or cockroachdb or ...
  • Thus the 'version' would not be the postgres version, but the version of the implementation. This would mean every feature would say, for each implementation, what version it is available at (if any).

Of course, there are questions about the resilience of these other implementations (e.g. adding integration testing with each one).

Thoughts?

kylepl added a commit to kylepl/pulumi-postgresql that referenced this issue Nov 7, 2023
Pulumi-wrapped-terraform postgres provider with a Cockroach DB backend.

Proper discussion is here:
cyrilgdn/terraform-provider-postgresql#372

To start using it locally, after installing the dependencies (I believe
just `go` and `pulumictl`: https://github.com/pulumi/pulumictl), with
the appropriate version number (I'm using v3.10.0):

`make provider && pulumi plugin rm resource postgresql && pulumi plugin
install resource postgresql v3.10.0 -f bin/pulumi-resource-postgresql`

The version number just needs to match what you are currently depending
on. `pulumi plugin ls` should show it.
@NidalShaterM
Copy link

@kylepl, did you publish a TF provider for this, I'm trying to replicate your changes and publish a new provider but I'm facing some issues with publishing it
@cyrilgdn could you tell me if you are planning to support cockroachDB soon?

@kylepl
Copy link
Author

kylepl commented Apr 29, 2024

@NidalShaterM - I did not, though I still use it locally. I never investigated how to publish it, so if it's actually the publishing part, I won't be much assistance, if it's something else, feel free to comment here.

@NidalShaterM
Copy link

NidalShaterM commented Apr 30, 2024

@NidalShaterM - I did not, though I still use it locally. I never investigated how to publish it, so if it's actually the publishing part, I won't be much assistance, if it's something else, feel free to comment here.

@kylepl, thanks for the quick response, I was trying to modify and use the provider with your changes, so I forked this repo and tried to use github directly as a source in the required_provider

terraform {
  required_providers {
    postgresql = {
      source = "github.com/nidalshaterm/postgresql"
      version="1.22.4"
    }
  }
}
terraform init

Initializing the backend...

Initializing provider plugins...
- Finding github.com/nidalshaterm/postgresql versions matching "1.22.4"...
╷
│ Error: Invalid provider registry host
│ 
│ The given source address "github.com/nidalshaterm/postgresql" specifies a GitHub repository rather than a Terraform provider. Refer to the documentation of the provider to find
│ the correct source address to use.

I tried also to use git submodules:

git submodule add https://github.com/NidalShaterM/terraform-provider-postgresql.git providers/postgressql
git submodule init  
 git submodule update

then

terraform {
 required_providers {
   postgresql = {
     source  = "./providers/postgressql"
     version="1.22.4"
   }
 }
}
terraform init                                                                                           

Initializing the backend...
Terraform encountered problems during initialisation, including problems
with the configuration, described below.

The Terraform configuration must be valid before initialization so that
Terraform can determine which modules and providers need to be installed.
╷
│ Error: Invalid provider source hostname
│ 
│   on main.tf line 4, in terraform:
│    4:       source  = "./providers/postgressql"
│ 
│ Invalid provider source hostname namespace "" in source "./providers/postgressql": hostname contains empty label (two consecutive periods)"
╵

╷
│ Error: Invalid provider source hostname
│ 
│   on main.tf line 4, in terraform:
│    4:       source  = "./providers/postgressql"
│ 
│ Invalid provider source hostname namespace "" in source "./providers/postgressql": hostname contains empty label (two consecutive periods)"
╵

even when publishing the provider and try to run apply (https://registry.terraform.io/providers/NidalShaterM/postgresql/latest)

terraform {
  required_providers {
    postgresql = {
      source = "NidalShaterM/postgresql"
      version = "1.22.5"
    }
  }
}
provider "postgresql" {
  host            = "127.0.0.1"
  port            = 26257
  database        = "defaultdb"
  username        = "demo"
  password        = "demo43554"
  sslmode         = "require"
  clientcert {
    cert = "CRT"
    key  = "KEY"
  }
}

resource "postgresql_role" "user1" {
  name     = "user1"
  password = "user1-password"
  login    = true
}
Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

postgresql_role.user1: Creating...
╷
│ Error: error detecting capabilities: error parsing version: Invalid character(s) found in major number "CCL"
│ 
│   with postgresql_role.user1,
│   on main.tf line 22, in resource "postgresql_role" "user1":
│   22: resource "postgresql_role" "user1" {
│ 
╵

so is there a faster way to test the CockroachDB changes?

@kylepl
Copy link
Author

kylepl commented Apr 30, 2024

So the last error message looks like progress - it is complaining that the version number is wrong (so it is running something), the CCL is from Cockroach.

So I'm actually using a wrapper over Terraform (Pulumi), so I'm not sure the syntax on the Terraform side, but one thing in my comment but not the in the merge (since it is outside of this repo itself, is):

The provider right now either deduces the version from SELECT version();, which does not work for Cockroach at least, but users can set expectedVersion. Ideally, parsing for Cockroach and other versions would be supported.

so I set expectedVersion = 15 in my configuration. So from my brief look at Terraform, try adding that after sslmode = "require" in your above block. Basically, it is telling the plug-in to not try to infer the version, but assume it is Postgres 15 (or whatever version you want it to assume).

@NidalShaterM
Copy link

NidalShaterM commented May 1, 2024

adding expected_version = 13 worked, thanks for your help

but when I tried to remove the user, I got the following error

postgresql_role.user1: Destroying... [id=user1]
╷
│ Error: could not drop owned by role user1: pq: unimplemented: drop owned by is not yet implemented

adding skip_reassign_owned resolved this, but still have other issues with granting and remove grants on users, I don't think it is a good idea to depend on this provider to manage CockroachDB, since I found other issues

@kylepl
Copy link
Author

kylepl commented May 1, 2024

Agreed that there is likely non-trivial development to make it work well, I hadn't, for instance, tried removing a user.

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

No branches or pull requests

2 participants