search

CheckPointSW / terraform-provider-checkpoint

Terraform provider for Check Point
https://www.terraform.io/docs/providers/checkpoint/
Mozilla Public License 2.0
28 stars 40 forks source link

Allow session file name choice to facilitate communication with multiple CMAs #83

Closed CNakai closed 3 years ago

CNakai commented 3 years ago

Previously, the provider would always write the session ID out to a file with the hardcoded name "sid.json". This becomes a problem when executing a Terraform plan that involves more than one Checkpoint CMA which is something that we must support at Mastercard. Specifically, the "sid.json" file created when communicating with the first CMA will be overwritten by the file created when communicating with the next CMA, and so on. This PR mainly addresses this issue.

Instead of hardcoding "sid.json" for the session file name, this PR allows for the specification of the session file name by parameterizing it just like the port, username, password, etc. However, in order to keep this change fully backward compatible, if this parameter is left unspecified it will default to "sid.json" as before.

This PR also includes one small "leave the code cleaner than you found it commit" which I hope is not too presumptuous. Additionally, please forgive the large amount of re-indentation in checkpoint/provider.go; my editor did it automatically and I didn't notice it until I had already committed. If it is a problem, please let me know and I can fix it.

b-diggity commented 3 years ago

Cool idea. We solved this by putting each domain into its own folder and using a matrix in GitHub actions to run the pipeline simultaneously against each domain folder. You can specify the working directory to be the current matrix item (the terraform code for the domain). This way, each domain will have a sid.json in its respective folder and they won't conflict. We also can check for changes first, before building the matrix, so we only run the pipeline on domain folders that had changes. I wonder if something like this would help you as well.

CNakai commented 3 years ago

Interesting! Thanks for sharing your solution, it may come in handy for us elsewhere or be useful to others, but I don't think we can do it that way due to the nature of our current project.

chkp-royl commented 3 years ago

Hi @CNakai , Thanks for your contribution for Check Point provider. We will review your suggestion and give back feedback as soon as possible.

Regards, Roy

chkp-royl commented 3 years ago

Hi @CNakai, In overall, the PR looks good and we can add support for user to choose session file name. We plan to release new version of Check Point provider soon and we are going to include your feature as well.

Regards, Roy

CNakai commented 3 years ago

Awesome, thanks! Is there anything else you need from me with regards to this PR?

chkp-alonshev commented 3 years ago

Thank you for submitting this PR, We fixed this issue on our provider's new version, V1.5.0, This version is now available for use. Thank you for your contribution!