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 Custom Endpoints to Logs Routing Service #5961

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

ianre
Copy link
Contributor

@ianre ianre commented Feb 3, 2025

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'
make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... | grep 'logsrouting') -v  -timeout 700m
=== RUN   TestAccIBMLogsRouterTargetsDataSourceBasic
--- PASS: TestAccIBMLogsRouterTargetsDataSourceBasic (74.38s)
=== RUN   TestDataSourceIBMLogsRouterTargetsTargetTypeToMap
--- PASS: TestDataSourceIBMLogsRouterTargetsTargetTypeToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTargetsTargetParametersTypeLogDnaToMap
--- PASS: TestDataSourceIBMLogsRouterTargetsTargetParametersTypeLogDnaToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTargetsTargetTypeLogDnaToMap
--- PASS: TestDataSourceIBMLogsRouterTargetsTargetTypeLogDnaToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTargetsTargetTypeLogsToMap
--- PASS: TestDataSourceIBMLogsRouterTargetsTargetTypeLogsToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTargetsTargetParametersTypeLogsToMap
--- PASS: TestDataSourceIBMLogsRouterTargetsTargetParametersTypeLogsToMap (0.00s)
=== RUN   TestAccIBMLogsRouterTenantsDataSourceBasic
--- PASS: TestAccIBMLogsRouterTenantsDataSourceBasic (74.28s)
=== RUN   TestDataSourceIBMLogsRouterTenantsTenantToMap
--- PASS: TestDataSourceIBMLogsRouterTenantsTenantToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTenantsTargetTypeToMap
--- PASS: TestDataSourceIBMLogsRouterTenantsTargetTypeToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTenantsTargetParametersTypeLogDnaToMap
--- PASS: TestDataSourceIBMLogsRouterTenantsTargetParametersTypeLogDnaToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTenantsTargetTypeLogDnaToMap
--- PASS: TestDataSourceIBMLogsRouterTenantsTargetTypeLogDnaToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTenantsTargetTypeLogsToMap
--- PASS: TestDataSourceIBMLogsRouterTenantsTargetTypeLogsToMap (0.00s)
=== RUN   TestDataSourceIBMLogsRouterTenantsTargetParametersTypeLogsToMap
--- PASS: TestDataSourceIBMLogsRouterTenantsTargetParametersTypeLogsToMap (0.00s)
=== RUN   TestAccIBMLogsRouterTenantBasic
--- PASS: TestAccIBMLogsRouterTenantBasic (78.74s)
=== RUN   TestAccIBMLogsRouterTenantAllArgs
--- PASS: TestAccIBMLogsRouterTenantAllArgs (80.57s)
=== RUN   TestResourceIBMLogsRouterTenantTargetTypeToMap
--- PASS: TestResourceIBMLogsRouterTenantTargetTypeToMap (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantTargetParametersTypeLogDnaToMap
--- PASS: TestResourceIBMLogsRouterTenantTargetParametersTypeLogDnaToMap (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantTargetTypeLogDnaToMap
--- PASS: TestResourceIBMLogsRouterTenantTargetTypeLogDnaToMap (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantTargetTypeLogsToMap
--- PASS: TestResourceIBMLogsRouterTenantTargetTypeLogsToMap (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantTargetParametersTypeLogsToMap
--- PASS: TestResourceIBMLogsRouterTenantTargetParametersTypeLogsToMap (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantMapToTargetTypePrototype
--- PASS: TestResourceIBMLogsRouterTenantMapToTargetTypePrototype (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantMapToTargetParametersTypeLogDnaPrototype
--- PASS: TestResourceIBMLogsRouterTenantMapToTargetParametersTypeLogDnaPrototype (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantMapToTargetTypePrototypeTargetTypeLogDnaPrototype
--- PASS: TestResourceIBMLogsRouterTenantMapToTargetTypePrototypeTargetTypeLogDnaPrototype (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantMapToTargetTypePrototypeTargetTypeLogsPrototype
--- PASS: TestResourceIBMLogsRouterTenantMapToTargetTypePrototypeTargetTypeLogsPrototype (0.00s)
=== RUN   TestResourceIBMLogsRouterTenantMapToTargetParametersTypeLogsPrototype
--- PASS: TestResourceIBMLogsRouterTenantMapToTargetParametersTypeLogsPrototype (0.00s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/logsrouting	311.426s

originalConfigServiceURL := logsRoutingClient.GetServiceURL()

f := conns.EnvFallBack([]string{"IBMCLOUD_ENDPOINTS_FILE_PATH", "IC_ENDPOINTS_FILE_PATH"}, "")
visibility := conns.EnvFallBack([]string{"IBMCLOUD_VISIBILITY", "IC_VISIBILITY"}, "")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BNPP will n't set any ENV variable they declare the file as part of provider bloack

Copy link
Collaborator

@hkantare hkantare Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just check for region in originalClient and replace with resource region if its n't empty and they are different

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I'm retrieving those env variables is in the custom endpoint documentation: https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/guides/custom-service-endpoints#2-define-service-endpoints-by-using-an-endpoints-file
I found clients can provide visibility and the endpoints file in the provider block or as env variables

provider "ibm" {
    # ... other provider configuration ...
    endpoints_file_path = "<file_path_to_endpoints_file_path>"
    visibility     = "<private_or_public>"
}

-or-

export IC_ENDPOINTS_FILE_PATH="<endpoint_value>"
export IC_VISIBILITY="<private_or_public>"

Can we just check for region in originalClient and replace with resource region if its n't empty and they are different

Yes, we could do that. However, if a client defines custom endpoints for all the regions, and they try to manage tenants in multiple regions within the same terraform file, how can I retrieve the custom endpoint from the endpoints file to update the URL?

It seems to me that utils.go must have access to the endpoints file so that it can use the custom URL for any new region it needs to manage

Copy link
Contributor Author

@ianre ianre Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just check for region in originalClient and replace with resource region if its n't empty and they are different

This would work if we only had to change the endpoint type (private or public) and the region. The tenant resource d *schema.ResourceData has the region, and public or private we can get from the current service url. However when a client provides a custom endpoints file, I think we have to check back in the file to retrieve endpoints for different regions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When user provides a custom endpoint URL also we would have already set the client URL in provider config it self
originalConfigServiceURL := logsRoutingClient.GetServiceURL() this should have the custom url.

The only thing we need to document is they need to have same mapping of region in custom/resource region atribute

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have simplified the code in utils.go to just determine the region and substitute it when needed. A client will be able to manage multiple tenants with custom endpoints in the same terraform file by declaring separate provider blocks. If they don't want to use custom endpoints, they don't need to declare separate provider blocks.

@hkantare hkantare merged commit 8255202 into IBM-Cloud:master Feb 4, 2025
1 check passed
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