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

CRUD operations of kafka_acl uses the APIKey in the credentials block instead of provider block #329

Open
jaylevin opened this issue Nov 14, 2023 · 5 comments · May be fixed by #328
Open

Comments

@jaylevin
Copy link

jaylevin commented Nov 14, 2023

Current behavior:
With the following API Key based provider authentication:

# Option #1: Manage multiple clusters in the same Terraform workspace
provider "confluent" {
  cloud_api_key    = var.confluent_cloud_api_key    # optionally use CONFLUENT_CLOUD_API_KEY env var
  cloud_api_secret = var.confluent_cloud_api_secret # optionally use CONFLUENT_CLOUD_API_SECRET env var
}

All CRUD operations on a kafka_acl results in the ACL's referenced credentials being used for authentication with Confluent instead of the cloud_api_key and cloud_api_secret defined in the provider's configuration block.

Expected behavior:

I would expect the creation, reading, deletion, or update of a kafka_acl to use the provider's configured APIKey/secret defined in the provider "confluent" { .. } block.

Observed behavior:
Because isKafkaMetadataSet checks for a cluster-specifickafka_rest_endpoint to be set in the provider's authentication block, the api key/secret defined in the provider's authentication block are ignored, and instead the credentials referenced in the actual ACL definition are used for the API operations to Confluent. This makes it impossible to implement this provider's use-case #1: "managing multiple confluent clusters in the same terraform workspace".

  • TLDR: The CRUD operations of a KafkaACL are using the API Key referenced in the kafka_acl's credentials block (not the static credentials defined in the provider block) when configuring the provider with the documented static credentials option #1:
Screenshot 2024-05-28 at 11 31 47 AM

I am wondering if there is a specific reason why the confluent provider is using the ACL's referenced credentials for authentication versus the credentials defined in the provider block? If there is no special reason for this, I think #328 would be a good solution for this problem.

@linouk23
Copy link
Collaborator

linouk23 commented Nov 26, 2023

Thanks for creating this issue @jaylevin!

I wonder what's the use case for having credentials in both provider block and resource itself? I agree that our documentation isn't very clear about it, but essentially we expect users to either use Option 1 or Option 2, and not combine them:
image

Lucatronlk pushed a commit to Lucatronlk/terraform-provider-confluent that referenced this issue Dec 28, 2023
@jaylevin
Copy link
Author

jaylevin commented Jan 16, 2024

I wonder what's the use case for having credentials in both provider block and resource itself? I agree that our documentation isn't very clear about it, but essentially we expect users to either use Option 1 or Option 2, and not combine them:

@linouk23 That is my understanding of the documentation as well and I'm not sure why it's been done this way.

The credentials referred to by the kafka_acl resource are the actual credentials associated with the kafka_apikey resource that is associated with the ACL resource. They are not (and shouldn't be IMO) used as credentials to authenticate the provider, but that is not the case with the current implementation of this provider.

When using Option 1, you specify a cloud API secret ID and Key. However, the code currently checks if the restEndpoint, apiKey, and apiKeySecret are present in the provider block before it uses the credentials in the provider block. See here.

Since the restEndpoint is not provided when using Option 1, the code defaults to using the credentials associated with the ACL's associated api_key resource: (see here).

I've opened a PR to fix this here. Looking forward to your review.

@jaylevin
Copy link
Author

Hi @linouk23, just a friendly ping here to see if you are still following this issue or if you had a chance to take a look at my open PR.

@shaafekhan
Copy link

Hi all, any update on this issue? @linouk23 @jaylevin

@jaylevin
Copy link
Author

jaylevin commented May 28, 2024

Hi @shaafekhan @linouk23, I am still waiting on an update from the maintainers as well

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