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

ENG-3601, ENG-4309: Repository binding and new sidecar params #25

Merged
merged 7 commits into from
Mar 11, 2021

Conversation

wcmjunior
Copy link
Contributor

Description of the change

  • Added a new resource to provide repo and sidecar binding functionality, allowing users to associate cyral_repository to cyral_sidecar resources.
  • Added a new aws_configuration section to cyral_sidecar resource to allow the definition of AWS parameters for sidecars.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • Jira issue referenced in commit message and/or PR title

Testing

Tested create, update, destroy and import commands.

"cyral_repository": resourceRepository(),
"cyral_sidecar": resourceSidecar(),
"cyral_repository": resourceRepository(),
"cyral_repository_binding": resourceRepositoryBinding(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Question, do we want to someday support the entire Cyral API? If so, it's possible to generate tons of Terraform resources from an OpenAPI doc. I started that off for the Terraform Vault Provider here and we could easily grab that code, modify it, and use it here too.

I suggest that because making Terraform resources can get fairly boring and repetitive once you've done one or two, and to cover lots of endpoints it's toil. If we instead work on generating the code, it keeps our lives more fun and gets a lot of bang for the buck on each engineering iteration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, to clarify, I'm not advocating that we generate this code, it looks great, just future code if we anticipate we'll be doing a lot of these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be an awesome idea. I'm not sure if we would be able to do it because our APIs behavior is not 100% uniform, so we have to make sure they behave the same prior automating the resource creation.

Copy link
Member

Choose a reason for hiding this comment

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

If possible, generating the code would be great indeed!

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be possible to generate most of the code, but then edit it by hand a little at the end to handle those little nuances.

@wcmjunior
Copy link
Contributor Author

This code has a known bug in cyral_datamap resource as described here: https://cyralinc.atlassian.net/browse/ENG-4331

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

Nice! Well done, Wilson!

@wcmjunior wcmjunior merged commit 49bb3b2 into main Mar 11, 2021
@wcmjunior wcmjunior deleted the feature/ENG-3601 branch March 11, 2021 00:10
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