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 azure network #377

Closed
wants to merge 6 commits into from
Closed

Feature/add azure network #377

wants to merge 6 commits into from

Conversation

MathieuBuisson
Copy link
Contributor

Hi,

Following #333 , which has introduced Azure support, the present PR adds coverage for network-related resources in the azure module :

  • Virtual Networks
  • Subnets
  • Network Security Groups
  • Public IPs

This contains only get methods for now.
If needed, methods to create and delete resources can be added in the future.

I opted to create a separate example module and a separate example test file from the existing one for Azure, because there are quite a few resources involved.
If you prefer to "merge" this example (and its test) with the existing one for Azure, I'm open to change that.

To view the end of the output of running the go test against the example test terraform_azure_network_example_test.go, please refer to this gist.

Issue link : #89

Thanks.

@brikis98
Copy link
Member

@MathieuBuisson Thanks for the PR! As Terratest has grown (we just passed 3,000 stars!), we've had to think more about what types of helper functions we want in the library to make sure it doesn't become so big as to be unusable. Please check out the updated contribution guidelines, especially the part about infrastructure and validation helpers. These Azure helpers meet requirements for "platforms" and "creating infrastructure," but what about "complexity" (i.e., are these just direct wrappers around the Azure SDK or are you doing something more complicated) and "popularity" (i.e., is this something most Azure/Terratest users will need or is it a relatively esoteric use case)?

@liafizan
Copy link

liafizan commented Feb 8, 2020

(i.e., is this something most Azure/Terratest users will need or is it a relatively esoteric use case)?

@brikis98 This would be a nice addition for azure. At present, I would need to wrap azure sdk with terratest to test the cases covered here. Would be good if we can have this feature as part of terratest package.
Also as we add more features for different clouds, I believe this is bound to become big. Perhaps we need to then target terratest for different cloud platforms separately.

@brikis98 brikis98 added the Azure label Jun 26, 2020
@brikis98
Copy link
Member

Please see #89 for the status on Terratest with Azure. In particular, we're starting some work around it, and when that work is done, we'll be able to come back to this PR!

@yorinasub17
Copy link
Contributor

Thanks for the initial contribution, and apologies we never got around to working with you to get this merged.

As @brikis98 mentioned, we are working out a new contribution standard for Azure specifically and actively working on getting that finalized. In the process, we merged a few PRs that appear to overlap with this in functionality (e.g., #631).

We also have a few more PRs from the Microsoft team we plan on prioritizing before officially announcing our new Azure contribution guidelines.

Given that, we will be closing this PR out, but feel free to contribute any missing functions from the package that you identify after the announcement.

Thanks for your patience, and again apologies that we never were able to bring this to merge.

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

Successfully merging this pull request may close these issues.

4 participants