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

Added virtual_network_rules for cosmosDB_account #1959

Closed
wants to merge 4 commits into from

Conversation

WodansSon
Copy link
Collaborator

Added is_virtual_network_filter and virtual_network_rules

(Fixes #1342 )

@ghost ghost added the size/L label Sep 20, 2018
@ghost ghost added the size/L label Sep 20, 2018
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Hi @jeffreyCline,

Thank you for the PR, aside from the comments I've left inline could we get a test for this?

azurerm/resource_arm_cosmos_db_account.go Show resolved Hide resolved
Default: false,
},

"virtual_network_rules": {
Copy link
Collaborator

@katbyte katbyte Sep 20, 2018

Choose a reason for hiding this comment

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

as discussed this could be simpler:

           "virtual_network_rule_subnet_ids": {
                Type:     schema.TypeSet,
				Optional: true,
                Elem:     &schema.Schema{Type: schema.TypeString},
				Set:      schema.HashString,
           }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that and it didn't work, I can try again...

@@ -335,6 +358,7 @@ func resourceArmCosmosDBAccountUpdate(d *schema.ResourceData, meta interface{})
client := meta.(*ArmClient).cosmosDBClient
ctx := meta.(*ArmClient).StopContext
log.Printf("[INFO] preparing arguments for AzureRM Cosmos DB Account update.")
log.Printf("[INFO] AzureRM Cosmos DB Account : %s", d)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we merge these two log statements?

is_virtual_network_filter_enabled = true

virtual_network_rules {
id = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/cosmosDBVNetRules/providers/Microsoft.Network/virtualNetworks/virtualNetwork1/subnets/testsubnet1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we use ${subnet.example.id} instead of an explicit id?

@WodansSon WodansSon added this to the 1.16.0 milestone Sep 20, 2018
@WodansSon WodansSon self-assigned this Sep 20, 2018
@pgavlin
Copy link
Contributor

pgavlin commented Sep 20, 2018

Looks like we raced to this: I've just opened #1961 to the same effect.

@katbyte
Copy link
Collaborator

katbyte commented Sep 21, 2018

Hi @pgavlin,

Indeed you both have!

#1961 also updates the datasource and has a test, so might i suggest we go with that one. What do you think @jeffreyCline?

@WodansSon
Copy link
Collaborator Author

@pgavlin @katbyte Agreed! I'll close this PR and we can use @pgavlin PR. Thanks for the PR @pgavlin

@WodansSon WodansSon closed this Sep 21, 2018
@WodansSon WodansSon deleted the u-cosmosdb_virtual-network-rule branch September 21, 2018 05:03
@ghost
Copy link

ghost commented Mar 6, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants