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

Add support for bearer token auth, additional arbitrary HTTP headers #81

Closed
wants to merge 2 commits into from

Conversation

benweint
Copy link
Contributor

This PR adds support for configuring this provider to use HTTP bearer authentication against the target Kafka Connect REST API endpoint, as an alternative to HTTP basic auth.

I believe this would address #41 and #33.

I've marked as draft because I've not actually tried this against a real cluster with bearer auth configured yet.

@MrLuje
Copy link

MrLuje commented Jul 1, 2022

I tested the extra_headers field to access a kafka connect behind a kubernetes api-server, it's working perfectly :)

provider "kafka-connect" {
   url = "https://admin-k8s.xxx/api/v1/namespaces/dev/services/http:cp-kafka-connect:8083/proxy"
+  extra_headers = {
+   "cookie" = "R_PCS=dark; R_REDIRECTED=true; R_LOCALE=en-us;...."
+  }
}

Without the extra_headers :

Error: Get connector : {"type":"error","status":"401","message":"Unauthorized 401: must authenticate"}

With the extra_headers :

Terraform will perform the following actions:

  # kafka-connect_connector.postgresql-outbox will be updated in-place
  ~ resource "kafka-connect_connector" "postgresql-outbox" {
      ~ config           = {
          - "slot.drop.on.stop"                                   = "true" -> null
            # (16 unchanged elements hidden)
        }
        id               = "postgresql-outbox"
        name             = "postgresql-outbox"
        # (1 unchanged attribute hidden)
    }

  # kafka-connect_connector.postgresql-outbox-dev will be updated in-place
  ~ resource "kafka-connect_connector" "postgresql-outbox-dev" {
      ~ config           = {
          + "connector.class"                                     = "io.debezium.connector.postgresql.PostgresConnector"
          .....
        }
        id               = "postgresql-outbox-dev"
        name             = "postgresql-outbox-dev"
        # (1 unchanged attribute hidden)
    }

Plan: 0 to add, 2 to change, 0 to destroy.

@MrLuje
Copy link

MrLuje commented Aug 17, 2022

@benweint Any chance to finalize this PR ?
I tested it on a real cluster (cf previous comment)

@benweint
Copy link
Contributor Author

Cool, I'm going to mark this as ready for review given that you've been able to test it @MrLuje!

@benweint benweint marked this pull request as ready for review August 17, 2022 15:03
@benweint
Copy link
Contributor Author

@Mongey would you mind taking a look when you have a moment?

@Mongey
Copy link
Owner

Mongey commented Aug 31, 2022

@benweint Why have special support for setting a bearer token vs just allowing setting of headers?

I think we should remove the bearer token, and rename extra_headers to headers unless there's a specific reason to support bearer token as it's own thing.

@benweint
Copy link
Contributor Author

benweint commented Aug 31, 2022

Why have special support for setting a bearer token vs just allowing setting of headers?

The main reason was that it's easier to inject the bearer token (which is just a string) via the KAFKA_CONNECT_BEARER_TOKEN env var then to figure out how to encode a set of key/value pairs for headers into an env var. We could have the env var be a JSON map or something, but it's perhaps not the most obvious thing.

FWIW, one could make the same argument about HTTP basic auth: it's ultimately just a header, so why bother having separate 'user' and 'password' env vars and provider configuration settings? I think it's a matter of convenience (having separate settings saves users of the provider from having to concatenate and base64-encode the username + password and put it into an HTTP header).

That being said, if you'd still prefer to keep the provider config interface generic, I'm happy to just switch to headers and punt on the question of allowing that setting to be configured via an env var!

@benweint
Copy link
Contributor Author

I opened #92 with the changes you suggested @Mongey - feel free to just pick whichever implementation you like better, depending on whether my argument above swayed you :)

@benweint
Copy link
Contributor Author

benweint commented Oct 1, 2022

@Mongey would you be open to merging this or #92 (which has the simplified version of this change that you suggested)?

@Mongey
Copy link
Owner

Mongey commented Oct 1, 2022

I've merged #92, thanks for the PR

@Mongey
Copy link
Owner

Mongey commented Oct 1, 2022

Going to suggest using a terraform variable, and using TF_VAR_KAFKA_CONNECT_BEARER_TOKEN for the env use case.

provider "kafka_connect"{
  headers = {
    "Authorization" = "Bearer ${var.kafka_connect_bearer_token}"
  }
}

@Mongey Mongey closed this Oct 1, 2022
@benweint
Copy link
Contributor Author

benweint commented Oct 1, 2022

Awesome, thank you!

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.

3 participants