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.

Sign up for GitHub

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

Jump to bottom

ACI - added secure environment variables #2024

Merged
merged 24 commits into from
Oct 18, 2018
Merged

ACI - added secure environment variables #2024

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d6b3e8b
Container Groups (ACI) - added secure environment variables
neilpeterson Sep 5, 2018
b8c71a8
Fix incomming merge conflict
neilpeterson Sep 7, 2018
8656731
Fix incomming merge conflict
neilpeterson Sep 7, 2018
a9bee2d
Updated variable / value names per PR review request
neilpeterson Oct 4, 2018
f97dac0
rebase with master
neilpeterson Oct 4, 2018
b9cb42a
Clena up vendor
neilpeterson Oct 4, 2018
882321b
Fixing up vendor.json
neilpeterson Oct 4, 2018
14c598b
Updated ACI documentation
neilpeterson Oct 9, 2018
62e3af4
Updated environmetn variabel append routine
neilpeterson Oct 9, 2018
a6d730f
Added slice capacity, removed tabs from test, and fixed vendor.json
neilpeterson Oct 11, 2018
331f71a
Removed unreachable return
neilpeterson Oct 11, 2018
c67f1ca
Fixing up vendor.json
neilpeterson Oct 11, 2018
692f497
Remove old version pacakge for container instance and clean up other …
metacpp Oct 12, 2018
d779d67
Fix the test case issue and styling issue.
metacpp Oct 16, 2018
aedf836
Code changes to fix test issues
WodansSon Oct 16, 2018
7858db1
Merge branch 'master' into container-group-add-secure-variables
WodansSon Oct 17, 2018
dc92a49
Update test to fix conflict
WodansSon Oct 17, 2018
45f92ba
Fixed casing per comment
WodansSon Oct 17, 2018
6b8e25c
Test: ignore the diffing for secure environment variables.
metacpp Oct 17, 2018
f912946
Add more diffing ignores for secure environment variables.
metacpp Oct 17, 2018
0f66616
Updated env var flatten function
WodansSon Oct 17, 2018
0e6d427
Fixed diff issue
WodansSon Oct 18, 2018
6705ded
Added secure_environment_variables validation to tests
WodansSon Oct 18, 2018
8896274
use name to look up index of old container to pull secure values over
katbyte Oct 18, 2018
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
2 changes: 1 addition & 1 deletion azurerm/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/Azure/azure-sdk-for-go/services/cdn/mgmt/2017-10-12/cdn"
"github.com/Azure/azure-sdk-for-go/services/cognitiveservices/mgmt/2017-04-18/cognitiveservices"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-06-01/compute"
"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-04-01/containerinstance"
"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-06-01/containerinstance"
"github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2017-10-01/containerregistry"
"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2018-03-31/containerservice"
"github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb"
Expand Down
2 changes: 0 additions & 2 deletions azurerm/resource_arm_api_management.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,6 @@ func flattenApiManagementHostnameConfigurations(input *[]apimanagement.HostnameC
"scm": scmResults,
},
}

return nil
}

func expandAzureRmApiManagementCertificates(d *schema.ResourceData) *[]apimanagement.CertificateConfiguration {
Expand Down
64 changes: 49 additions & 15 deletions azurerm/resource_arm_container_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"log"
"strings"

"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-04-01/containerinstance"
"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-06-01/containerinstance"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
Expand Down Expand Up @@ -34,11 +34,11 @@ func resourceArmContainerGroup() *schema.Resource {
"ip_address_type": {
Type: schema.TypeString,
Optional: true,
Default: "Public",
Default: "public",
WodansSon marked this conversation as resolved.
Show resolved Hide resolved
ForceNew: true,
DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
ValidateFunc: validation.StringInSlice([]string{
"Public",
"public",
WodansSon marked this conversation as resolved.
Show resolved Hide resolved
}, true),
},

Expand Down Expand Up @@ -166,21 +166,24 @@ func resourceArmContainerGroup() *schema.Resource {
"environment_variables": {
Type: schema.TypeMap,
Optional: true,
ForceNew: true,
},

"sensitive_environment_variables": {
Copy link
Contributor

@metacpp metacpp Oct 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

We may need to align with Azure documentation:
https://docs.microsoft.com/en-us/azure/container-instances/container-instances-environment-variables#secure-values

Copy link
Collaborator

Choose a reason for hiding this comment

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

I disagree, as this implies a higher level of security that isn't there since the values are persisted in the HCL file plan test. Since it is not encrypted I would feel better if we would expose it as sensitive and in the code pass it as secureValue as required by the API.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there was a misunderstanding that this wasn't surfacing something the API here internally. While these variable are not "secure" in Terraform, they are more secure in the container group and that is the name used in the API, so we should probably match it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, I have reverted back to using the secure name instead of sensitive.

Type: schema.TypeMap,
Optional: true,
Sensitive: true,
},

"command": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Computed: true,
Deprecated: "Use `commands` instead.",
},

"commands": {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
Expand Down Expand Up @@ -564,10 +567,26 @@ func expandContainerGroupContainers(d *schema.ResourceData) (*[]containerinstanc
containerGroupPorts = append(containerGroupPorts, containerGroupPort)
}

// Set both sensitive and non-secure environment variables
var envVars *[]containerinstance.EnvironmentVariable
var secEnvVars *[]containerinstance.EnvironmentVariable

// Expand environment_variables into slice
if v, ok := data["environment_variables"]; ok {
container.EnvironmentVariables = expandContainerEnvironmentVariables(v)
envVars = expandContainerEnvironmentVariables(v, false)
}

// Expand sensitive_environment_variables into slice
if v, ok := data["sensitive_environment_variables"]; ok {
secEnvVars = expandContainerEnvironmentVariables(v, true)
}

// Combine environment variabel slices
*envVars = append(*envVars, *secEnvVars...)

// Set both secure and non secure environment variables
container.EnvironmentVariables = envVars

if v, ok := data["commands"]; ok {
c := v.([]interface{})
command := make([]string, 0)
Expand Down Expand Up @@ -599,18 +618,33 @@ func expandContainerGroupContainers(d *schema.ResourceData) (*[]containerinstanc
return &containers, &containerGroupPorts, &containerGroupVolumes
}

func expandContainerEnvironmentVariables(input interface{}) *[]containerinstance.EnvironmentVariable {
func expandContainerEnvironmentVariables(input interface{}, secure bool) *[]containerinstance.EnvironmentVariable {

envVars := input.(map[string]interface{})
output := make([]containerinstance.EnvironmentVariable, 0)
output := make([]containerinstance.EnvironmentVariable, 0, len(envVars))

for k, v := range envVars {
ev := containerinstance.EnvironmentVariable{
Name: utils.String(k),
Value: utils.String(v.(string)),
if secure == true {

for k, v := range envVars {
ev := containerinstance.EnvironmentVariable{
Name: utils.String(k),
SecureValue: utils.String(v.(string)),
}

output = append(output, ev)
}
output = append(output, ev)
}

} else {

for k, v := range envVars {
ev := containerinstance.EnvironmentVariable{
Name: utils.String(k),
Value: utils.String(v.(string)),
}

output = append(output, ev)
}
}
return &output
}

Expand Down
121 changes: 67 additions & 54 deletions azurerm/resource_arm_container_group_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ func TestAccAzureRMContainerGroup_linuxComplete(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.%", "2"),
resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.foo", "bar"),
resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.foo1", "bar1"),
resource.TestCheckResourceAttr(resourceName, "container.0.sensitive_environment_variables.%", "0"),
resource.TestCheckResourceAttr(resourceName, "container.0.volume.#", "1"),
resource.TestCheckResourceAttr(resourceName, "container.0.volume.0.mount_path", "/aci/logs"),
resource.TestCheckResourceAttr(resourceName, "container.0.volume.0.name", "logs"),
Expand Down Expand Up @@ -245,6 +246,7 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.%", "2"),
resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.foo", "bar"),
resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.foo1", "bar1"),
resource.TestCheckResourceAttr(resourceName, "container.0.sensitive_environment_variables.%", "0"),
resource.TestCheckResourceAttr(resourceName, "os_type", "Windows"),
resource.TestCheckResourceAttr(resourceName, "restart_policy", "Never"),
),
Expand Down Expand Up @@ -308,7 +310,7 @@ resource "azurerm_container_group" "test" {
memory = "0.5"
port = "80"
}

image_registry_credential {
server = "hub.docker.com"
username = "yourusername"
Expand Down Expand Up @@ -356,7 +358,7 @@ resource "azurerm_container_group" "test" {
memory = "0.5"
port = "80"
}

image_registry_credential {
server = "hub.docker.com"
username = "updatedusername"
Expand Down Expand Up @@ -465,11 +467,17 @@ resource "azurerm_container_group" "test" {
memory = "3.5"
port = "80"

environment_variables {
"foo" = "bar"
"foo1" = "bar1"
}
commands = ["cmd.exe", "echo", "hi"]
environment_variables {
"foo" = "bar"
"foo1" = "bar1"
}

sensitive_environment_variables {
"secureFoo" = "secureBar"
"secureFoo1" = "secureBar1"
}

commands = ["cmd.exe", "echo", "hi"]
}

tags {
Expand All @@ -482,66 +490,71 @@ resource "azurerm_container_group" "test" {
func testAccAzureRMContainerGroup_linuxComplete(ri int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-%d"
location = "%s"
name = "acctestRG-%d"
location = "%s"
}

resource "azurerm_storage_account" "test" {
name = "accsa%d"
resource_group_name = "${azurerm_resource_group.test.name}"
location = "${azurerm_resource_group.test.location}"
account_tier = "Standard"
account_replication_type = "LRS"
name = "accsa%d"
resource_group_name = "${azurerm_resource_group.test.name}"
location = "${azurerm_resource_group.test.location}"
account_tier = "Standard"
account_replication_type = "LRS"
}

resource "azurerm_storage_share" "test" {
name = "acctestss-%d"
name = "acctestss-%d"

resource_group_name = "${azurerm_resource_group.test.name}"
storage_account_name = "${azurerm_storage_account.test.name}"
resource_group_name = "${azurerm_resource_group.test.name}"
storage_account_name = "${azurerm_storage_account.test.name}"

quota = 50
quota = 50
}

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"
dns_name_label = "acctestcontainergroup-%d"
os_type = "linux"
restart_policy = "OnFailure"

container {
name = "hf"
image = "seanmckenna/aci-hellofiles"
cpu = "1"
memory = "1.5"

port = "80"
protocol = "TCP"

volume {
name = "logs"
mount_path = "/aci/logs"
read_only = false
share_name = "${azurerm_storage_share.test.name}"

storage_account_name = "${azurerm_storage_account.test.name}"
storage_account_key = "${azurerm_storage_account.test.primary_access_key}"
}

environment_variables {
"foo" = "bar"
"foo1" = "bar1"
}
name = "acctestcontainergroup-%d"
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
ip_address_type = "public"
dns_name_label = "acctestcontainergroup-%d"
os_type = "linux"
restart_policy = "OnFailure"

commands = ["/bin/bash", "-c", "ls"]
}
container {
name = "hf"
image = "seanmckenna/aci-hellofiles"
cpu = "1"
memory = "1.5"

port = "80"
protocol = "TCP"

volume {
name = "logs"
mount_path = "/aci/logs"
read_only = false
share_name = "${azurerm_storage_share.test.name}"

storage_account_name = "${azurerm_storage_account.test.name}"
storage_account_key = "${azurerm_storage_account.test.primary_access_key}"
}

environment_variables {
"foo" = "bar"
"foo1" = "bar1"
}

sensitive_environment_variables {
"secureFoo" = "secureBar"
"secureFoo1" = "secureBar1"
}

commands = ["/bin/bash", "-c", "ls"]
}

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