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

Join endpoint_configuration and endpoints into single field and m… #273

Merged
merged 6 commits into from
Apr 3, 2025

Conversation

whites11
Copy link
Contributor

@whites11 whites11 commented Apr 3, 2025

…ake changes to the plan more understandable

Towards: #267

but a general improvement overall.

This PR aims at simplifying and improving the management of endpoints (read protocols) in a clickhouse service in both directions (input or enable/disable and output to get the host and port for each endpoint).

The change is twofold:

  1. remove the endpoint_configuration attribute and move endpoint config to the endpoints attribute.
  2. change the endpoints attribute from being a list to a map.

Example.

Before this change, to enable the mysql endpoint (the only configurable one) one needed to have this in .tf file

resource "clickhouse_service" "service" {
  ...
  endpoints_configuration = {
    mysql = {
      enabled = true
    }
  }
  ...
}

and the output would have been something like this:

endpoints = [
  {
    protocol = "https"
    host = "ql5ek38hzz.us-east-2.aws.clickhouse.cloud"
    port: 8443
  },
  {
    protocol: "mysql"
    host: "ql5ek38hzz.us-east-2.aws.clickhouse.cloud",
    port: 3306
  },
  ...
]

After this change, users need to use the endpoints field for both enabling the mysql endpoint (and possibly others in the future) and get the hostnames, in a better format like this:

resource "clickhouse_service" "service" {
...
  endpoints = {
    mysql = {
      enabled = true
    }
  }
...
}

and the outputs

endpoints = {
  "https": {
    "host": "ql5ek38hzz.us-east-2.aws.clickhouse.cloud",
    "port": 8443
  },
  "mysql": {
    "enabled": false,
    "host": null,
    "port": null
  },
  "nativesecure": {
    "host": "ql5ek38hzz.us-east-2.aws.clickhouse.cloud",
    "port": 9440
  }
}

the main advantage of this change is that we can have a more fine grained control over the plan when changing endpoint settings, and avoid having all endpoints show up as "known after apply" everytime there was a change.
This has been a rightful request from a few customers.

…ake changes to the plan more understandable
@whites11 whites11 requested a review from a team as a code owner April 3, 2025 10:22
@whites11 whites11 self-assigned this Apr 3, 2025
whites11 added 4 commits April 3, 2025 12:26
…ake changes to the plan more understandable
…ake changes to the plan more understandable
…ake changes to the plan more understandable
…ake changes to the plan more understandable
…ake changes to the plan more understandable
@whites11 whites11 merged commit 2c791f0 into main Apr 3, 2025
39 checks passed
@whites11 whites11 deleted the endpoints branch April 3, 2025 15:02
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

Successfully merging this pull request may close these issues.

2 participants