trendmicro / cloudone-filestorage-plugins

Trend Micro Cloud One File Storage Security plugins reference code.
https://cloudone.trendmicro.com
Apache License 2.0
42 stars 51 forks source link

feat: Add GCP Promote and Quarantine Plugin #84

Closed andrew-c-lee closed 2 years ago

andrew-c-lee commented 2 years ago

Add GCP Promote and Quarantine Plugin

Change Summary

Add GCP promte and quarantine plugin

PR Checklist

Other Notes

If open in cloudshell button needed to test use this link

andrew-c-lee commented 2 years ago

Hi @davidblanchard123, Cloud you help review the wording in README.md, deploy-tutorial.md, and description in variable.tf ? Thanks!

trend-laurence-cheng commented 2 years ago

Do we need to provide a step in deploy-tutorial.md and README.md that the user needs to copy main.auto.tfvars.example and rename it as main.auto.tfvars? And they also have to fill the correct values in the file.

andrew-c-lee commented 2 years ago

Do we need to provide a step in deploy-tutorial.md and README.md that the user needs to copy main.auto.tfvars.example and rename it as main.auto.tfvars? And they also have to fill the correct values in the file.

If the user hit terraform apply without main.auto.tfvars the terraform CLI would instruct the user to fill in to variables one by one. But I think it would be nice if the instruction on main.auto.tfvars is provided would be better for the user.

davidblanchard123 commented 2 years ago

Overall, everything looks good. A minor numbering issue and questions on the main.auto.tfvars.example file and “Delete the function” step.

The question about the main.auto.tfvars.example file. Am I supposed to use it? It looks like it would save me time by creating a file that I can re-use without having to file in the variable each time. I’m assuming it would be renamed to remove the “.example” but then how and where I apply it.

You may want to add a section to both the readme.md and deploy-tutorial.md files. It would go something like this:

main.auto.tfvars file

You can use the main.auto.tfvars file to upload your variables instead of filling them in individually. Once you have created and saved the file, you can reuse the file rather than fill in the variables every time.

To create the file, add the variables to the “main.auto.tvars.example” file and then save the file as “main.auto.tfvars”

Readme.md

Overall looks good.

Local Machine section

However, please number the steps rather than just using “1.” This makes it easier when looking at the rich diff.

Line 37. Deploy the function and add the variables following the cli (or upload the main.auto.tfvars file).

Is the “Delete the function” step part of the deployment? Is it automatically deleted at the end of the process, or is it a separate manual function? If this is a separate manual function, it should be broken out under it separate heading.

Deploy-tutorial.md

Overall looks good

Deploy promote and quarantine function.

Please number the steps concurrently. In the source diff, the numbering goes 1,1,2,3; In the rich diff goes 1,2,3,4. It makes it hard to determine if steps are main steps or subsets.

Line 33. Deploy the function and add the variables following the cli (or upload the main.auto.tfvars file).

Is the “Delete the function” step part of the deployment? Is it automatically deleted at the end of the process, or is it a separate manual function? If this is a separate manual function, it should be broken out under it separate heading.

Variables.tf

Looks good

andrew-c-lee commented 2 years ago

Hi @davidblanchard123, I had updated the wording in the documents. Cloud you help to review the wording again? Thanks!