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

[FEATURE] Add destination resource #931

Open
MarcusNotheis opened this issue Oct 20, 2024 · 9 comments
Open

[FEATURE] Add destination resource #931

MarcusNotheis opened this issue Oct 20, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@MarcusNotheis
Copy link

MarcusNotheis commented Oct 20, 2024

What area do you want to see improved?

terraform provider

Is your feature request related to a problem? Please describe.

We would like to manage our destinations in an instance of the BTP Destination Service via terraform. Therefore, we would like to create, update and delete destinations on both subaccount and instance level.

Describe the solution you would like

Manage destinations via terraform with a btp_destination resource:

resource "btp_destination" "my_serviceinstance" {
  service_instance_id = "uuid-goes-here"
  service_key         = "terraform-automation"
  name                = "my-destination"
  type                = "HTTP" # enum with other types
  proxy_type          = "Internet"
  description         = "Destination to sap.com"
  url                 = "https://sap.com"
  authentication      = "NoAuthentication" # enum with other auth types
  # if authentication would be OAuth2JWTBearer, then authentication_properties would need to be passed as well
  authentication_properties = jsondecode({
    tokenServiceURLType = "Dedicated"
    clientId : data.some_secure_source_like_vault.clientId
    clientSecret : data.some_secure_source_like_vault.clientSecret
    tokenServiceURL : "https://..."
  })
  additional_properties = jsondecode({
    foo = "bar"
  })
}

Maybe it would also make sense to model each authentication type as a dedicated resource to make sure required parameters are passed during terraform plan.

Based on the instance id and the service key, the BTP provider could read the required credentials and create/update/delete the destination via API calls to the destination service.

Describe alternatives you have considered

We might use the cloudfoundry_service_instance from the sap/cloudfoundry provider and pass all destinations as init_data, but this JSON would become huge when maintaining lots of different destinations.

Additional context

No response

@MarcusNotheis MarcusNotheis added enhancement New feature or request pending-decision This issue has not been accepted for implementation nor rejected. It's still open to discussion. labels Oct 20, 2024
Copy link

Thanks for the feature request. We evaluate it and update the issue accordingly.

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post 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 issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@lechnerc77
Copy link
Member

lechnerc77 commented Oct 20, 2024

@MarcusNotheis we are working on providing a dedicated resource for destinations as proposed in you feature request. As this is not only a topic of the Terraform provider, some pre-work needs to be done before offering a new resource in the provider. As of now we cannot give an ETA for the resource, however we ca already say that this won't land in the the first half of 2025.

Concerning the current options you have: as you rightfully stated you can use the service instance resource for Cloud Foundry or the service instance resource for SAP BTP. However, please be aware of the restrictions that these resources for service instances with dedicated (= JSON encoded) parameters have concerning drift detection and import.

@lechnerc77 lechnerc77 removed the pending-decision This issue has not been accepted for implementation nor rejected. It's still open to discussion. label Oct 20, 2024
@CoKueb
Copy link

CoKueb commented Oct 23, 2024

Hey @lechnerc77,
we just had some internal discussions.
Would you be willing to accept a contribution to this repository? We could try to build the required feature internally and then open a pull request.
If needed we could also setup a short call discuss the process and try to define requirements and acceptance criteria together.

BR,
Corvin

@lechnerc77
Copy link
Member

@CoKueb I will ping you. However the functionality must be added first the BTP CLI before we can use it in the provider

@ptesny
Copy link

ptesny commented Nov 11, 2024

Hello, it is fairly easy to provide destination definitions, including the with the certificates, during a destination resource creation. All by the book.
Please have a look at:

https://github.com/quovadis-btp/btp-automation/blob/main/btp-context/bootstrap-context/modules/custom-idp/bootstrap_destination_trust.tf#L33

https://github.com/quovadis-btp/btp-automation/blob/main/btp-context/provider-context/modules/sap-hana-cloud/main.tf#L264

You have can have subaccount and/or instance level destinations as well.
PS.
A few pics from a provider context BTP subaccount:

image image

@MarcusNotheis @lechnerc77 @CoKueb

@lechnerc77
Copy link
Member

@ptesny : I agree that the provisioning via service instances works. However, this is not the preferred way from the perspective of destinations and comes with some short comings (no import possible, limited read capability of parameters) that would be mitigated by the dedicated resource (i.e. by the corresponding API)

@MarcusNotheis
Copy link
Author

In addition to what @lechnerc77 said, it's also not possible to remove destinations which are not longer needed with the approach using init_data

@ptesny
Copy link

ptesny commented Nov 11, 2024

In addition to what @lechnerc77 said, it's also not possible to remove destinations which are not longer needed with the approach using init_data

Are you sure ? If it were not possible this well-documented init_data approach would have been very much useless ?
I used to do it from a BTP cockpit in the past.
In the code snippet I provided the set of definitions is fixed but not their content.... different attributes like secrets, x509 certificates etc can and are being rotated... If you need to remove a destination you just remove it from init_data. Or am I wrong ?

PS. https://help.sap.com/docs/connectivity/sap-btp-connectivity-cf/use-config-json-to-create-or-update-destination-service-instance

Configuration as code (CaC) with destinations.

@ptesny
Copy link

ptesny commented Nov 11, 2024

@ptesny : I agree that the provisioning via service instances works. However, this is not the preferred way from the perspective of destinations and comes with some short comings (no import possible, limited read capability of parameters) that would be mitigated by the dedicated resource (i.e. by the corresponding API)

Indeed. However, on a side note, I have conceptualised and developed the following blueprint:
Configuration as code (CaC) with destinations.

And, very much arguably, the biggest shortcoming I wanted to address with the above blueprint is the lack of an intrinsic BTP CLI command to assist in creation of destinations from service bindings.

With this solved I am also providing a destination-service bootstrap destinations definitions - against a dest service binding(s).

Subsequently, that makes fairly easy to manage destinations real estate via the destination service REST APIs, including part of a tf configuration

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

No branches or pull requests

4 participants