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

resource/container_group: add registry credential #1529

Merged
merged 7 commits into from
Jul 16, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 95 additions & 2 deletions azurerm/resource_arm_container_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,37 @@ func resourceArmContainerGroup() *schema.Resource {
}, true),
},

"image_registry_credential": {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"server": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.NoZeroValues,
ForceNew: true,
},

"username": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.NoZeroValues,
ForceNew: true,
},

"password": {
Type: schema.TypeString,
Required: true,
Sensitive: true,
ValidateFunc: validation.NoZeroValues,
ForceNew: true,
},
},
},
},

"tags": tagsForceNewSchema(),

"restart_policy": {
Expand Down Expand Up @@ -218,8 +249,9 @@ func resourceArmContainerGroupCreate(d *schema.ResourceData, meta interface{}) e
Type: &IPAddressType,
Ports: containerGroupPorts,
},
OsType: containerinstance.OperatingSystemTypes(OSType),
Volumes: containerGroupVolumes,
OsType: containerinstance.OperatingSystemTypes(OSType),
Volumes: containerGroupVolumes,
ImageRegistryCredentials: expandContainerImageRegistryCredentials(d),
},
}

Expand Down Expand Up @@ -277,6 +309,10 @@ func resourceArmContainerGroupRead(d *schema.ResourceData, meta interface{}) err
}
flattenAndSetTags(d, resp.Tags)

if imageRegCreds := flattenContainerImageRegistryCredentials(d, resp.ImageRegistryCredentials); imageRegCreds != nil {
d.Set("image_registry_credential", imageRegCreds)
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 check for an error here? like:

if err := d.Set("capabilities", flattenAzureRmCosmosDBAccountCapabilities(resp.Capabilities)); err != nil {
		return fmt.Errorf("Error setting `capabilities`: %+v", err)
	}

d.Set() sill handle a nil value correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, make sense I'll fix this up

}

d.Set("os_type", string(resp.OsType))
if address := resp.IPAddress; address != nil {
d.Set("ip_address_type", address.Type)
Expand Down Expand Up @@ -531,6 +567,63 @@ func expandContainerEnvironmentVariables(input interface{}) *[]containerinstance
return &output
}

func expandContainerImageRegistryCredentials(d *schema.ResourceData) *[]containerinstance.ImageRegistryCredential {
credsRaw := d.Get("image_registry_credential").([]interface{})
if len(credsRaw) == 0 {
return nil
}

output := make([]containerinstance.ImageRegistryCredential, 0, len(credsRaw))

for _, c := range credsRaw {
credConfig := c.(map[string]interface{})

server := credConfig["server"].(string)
username := credConfig["username"].(string)
password := credConfig["password"].(string)

output = append(output, containerinstance.ImageRegistryCredential{
Server: &server,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very minor but these could be assigned inline via utils.String()

Server: utils.String(credConfig["username"].(string)),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah much nicer, I thought about bringing in the to package from Autorest as has lots of these funcs too didn't realize you already had one.

Password: &password,
Username: &username,
})
}

return &output
}

func flattenContainerImageRegistryCredentials(d *schema.ResourceData, credsPtr *[]containerinstance.ImageRegistryCredential) []interface{} {
if credsPtr == nil {
return nil
}
configsOld := d.Get("image_registry_credential").([]interface{})

creds := *credsPtr
output := make([]interface{}, len(creds))
for _, cred := range creds {
credConfig := make(map[string]interface{})
if cred.Server != nil {
credConfig["server"] = *cred.Server
}
if cred.Username != nil {
credConfig["username"] = *cred.Username
}

for i, configsOld := range configsOld {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure, but i presume the ordering is consistent so you could get the old values this way:

for i, oldIrcConfig := range ircConfigsOld {
...

    if v, ok := d.GetOk(fmt.Sprintf("image_registry_credential.%d.password", i)); ok {
        block["password"] = v.(string)
    }
}

Copy link
Contributor Author

@lawrencegripper lawrencegripper Jul 11, 2018

Choose a reason for hiding this comment

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

I'm a little uncertain on this one so was being careful. Does the ordering of elements relate to the ordering of the elements in HCL? So for example if the user update the password element in their template but also moved the credential block ahead of another existing block would that affect this approach (making sure the server name matches)? Feels like an edge case either way round.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the order of the list directly relates to the order of the elements in HCL. So if the user does as you describe, the Apply would be sending a new list anyways, and the subsequent read would be post Update. unless azure returns things in a different order I think it should be fine. You could always write a test to confirm 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, makes sense to me. Will also check with the test just to be on the safe side.

data := configsOld.(map[string]interface{})
oldServer := data["server"].(string)
if cred.Server != nil && *cred.Server == oldServer {
if v, ok := d.GetOk(fmt.Sprintf("image_registry_credential.%d.password", i)); ok {
credConfig["password"] = v.(string)
}
}
}

output = append(output, credConfig)
}
return output
}

func expandContainerVolumes(input interface{}) (*[]containerinstance.VolumeMount, *[]containerinstance.Volume) {
volumesRaw := input.([]interface{})

Expand Down
78 changes: 77 additions & 1 deletion azurerm/resource_arm_container_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,40 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

func TestAccAzureRMContainerGroup_linuxBasic(t *testing.T) {
func TestAccAzureRMContainerGroup_imageRegistryCredentials(t *testing.T) {
resourceName := "azurerm_container_group.test"
ri := acctest.RandInt()

config := testAccAzureRMContainerGroup_linuxBasic(ri, testLocation())

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMContainerGroupDestroy,
Steps: []resource.TestStep{
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMContainerGroupExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.#:", "2"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.0.server", "hub.docker.com"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.0.username", "yourusername"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.0.password:", "yourpassword"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.1.server", "mine.acr.io"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.1.username", "acrusername"),
resource.TestCheckResourceAttr(resourceName, "image_registry_credential.1.password:", "acrpassword"),
),
},
},
})
}

func TestAccAzureRMContainerGroup_linuxBasic(t *testing.T) {
resourceName := "azurerm_container_group.test"
ri := acctest.RandInt()

config := testAccAzureRMContainerGroup_imageRegistryCredentials(ri, testLocation())
Copy link
Contributor

Choose a reason for hiding this comment

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

the tests are failing since this has been updated but the counts are incorrect; going to push a test to revert this to TestAccAzureRMContainerGroup_linuxBasic


resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Expand Down Expand Up @@ -176,6 +204,54 @@ resource "azurerm_container_group" "test" {
`, ri, location, ri)
}

func testAccAzureRMContainerGroup_imageRegistryCredentials(ri int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-%d"
location = "%s"
}

resource "azurerm_container_group" "test" {
name = "acctestcontainergroup-%d"
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
ip_address_type = "public"
os_type = "linux"

container {
name = "hw"
image = "microsoft/aci-helloworld:latest"
cpu = "0.5"
memory = "0.5"
port = "80"
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 fix the alignment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woop will do

}

image_registry_credential {
server = "hub.docker.com"
username = "yourusername"
password = "yourpassword"
}

image_registry_credential {
server = "mine.acr.io"
username = "acrusername"
password = "acrpassword"
}

container {
name = "sidecar"
image = "microsoft/aci-tutorial-sidecar"
cpu = "0.5"
memory = "0.5"
}

tags {
environment = "Testing"
}
}
`, ri, location, ri)
}

func testAccAzureRMContainerGroup_linuxBasicUpdated(ri int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
Expand Down
37 changes: 37 additions & 0 deletions examples/aci-image-registry-credentials/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
resource "azurerm_resource_group" "aci-rg" {
name = "aci-test-creds"
location = "west us"
}

resource "azurerm_container_group" "aci-test" {
name = "my-aci-hw"
location = "${azurerm_resource_group.aci-rg.location}"
resource_group_name = "${azurerm_resource_group.aci-rg.name}"
ip_address_type = "public"
os_type = "linux"

image_registry_credential {
server = "hub.docker.com"
username = "yourusername1"
password = "yourpassword"
}

container {
name = "hw"
image = "microsoft/aci-helloworld:latest"
cpu = "0.5"
memory = "1.5"
port = "80"
}

container {
name = "sidecar"
image = "microsoft/aci-tutorial-sidecar"
cpu = "0.5"
memory = "1.5"
}

tags {
environment = "testing"
}
}
10 changes: 10 additions & 0 deletions website/docs/r/container_group.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ The following arguments are supported:

* `restart_policy` - (Optional) Restart policy for the container group. Allowed values are `Always`, `Never`, `OnFailure`. Defaults to `Always`.

* `image_registry_credential` - (Optional) Set image registry credentials for the group as documented in the `image_registry_credential` block below

* `container` - (Required) The definition of a container that is part of the group as documented in the `container` block below. Changing this forces a new resource to be created.

~> **Note:** if `os_type` is set to `Windows` currently only a single `container` block is supported.
Expand Down Expand Up @@ -135,6 +137,14 @@ The `volume` block supports:

* `share_name` - (Required) The Azure storage share that is to be mounted as a volume. This must be created on the storage account specified as above. Changing this forces a new resource to be created.

The `image_registry_credential` block supports:

* `username` - (Required) The username with which to connect to the registry.

* `password` - (Required) The password with which to connect to the registry.

* `server` - (Required) The address to use to connect to the registry without protocol ("https"/"http"). For example: "myacr.acr.io"

## Attributes Reference

The following attributes are exported:
Expand Down