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

Minor provider field changes #1

Merged
merged 4 commits into from
Feb 19, 2023
Merged

Conversation

Rwwn
Copy link
Contributor

@Rwwn Rwwn commented Feb 15, 2023

First off I'd like to thank you for creating this provider. The fact it lets me manage elastic agent configurations is invaluable. I've yet to see any other config as code tool allow that in GoCD. Please keep it up.

I've got three changes I'd like to suggest here:

  1. A small fix to the log level defaults which was using the password field's default by mistake.
  2. Changing the default for ca_file to nil and making it optional. If I'm not mistaken, the current default was inadvertently causing this if statement to evaluate to true, so the client would try to use the "some_ca_context" default value as a certificate, causing it to fail. I believe the desired behaviour is for it to evaluate to false, and skip strict TLS checking. This fixes the errorx509: certificate signed by unknown authority I'm seeing when trying to use the provider without a ca_cert field.
  3. Changing the propertiesSchemaResource so that changes to the settings of cluster profiles and elastic agent profiles doesn't force a rebuild of the profiles. There's no reason these can't be updated in place, and replacing them each time is a problem since they can't be deleted if they're associated with a pipeline.

I haven't tested these changes as I'm not sure on how I'd do that exactly. I've tested this by using a local version of the provider with my changes, and it's all working, at least with my configuration.

This is my first time contributing to a Terraform provider. I'm happy to take any suggestions if you have them.

@nikhilsbhat
Copy link
Owner

@Rwwn good to know that this provider is helpful for you.

The changes looks good too. I will test it once and merge it ASAP.

@nikhilsbhat nikhilsbhat merged commit e07b43a into nikhilsbhat:main Feb 19, 2023
@nikhilsbhat
Copy link
Owner

Thank you for this contribution @Rwwn

@Rwwn Rwwn deleted the provider_update branch February 20, 2023 09:07
@Rwwn
Copy link
Contributor Author

Rwwn commented Feb 20, 2023

Thanks for merging it @nikhilsbhat. Will it be included in a release soon so I can start using it?

@nikhilsbhat
Copy link
Owner

yes @Rwwn, I'm working on it. Fixing few more feedback that I received.

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.

None yet

2 participants