Add secret_namespace to volume_source azure_file #1204

bancek · 2021-03-17T21:50:36Z

Description

This adds support for setting secret_namespace argument for azure_file volume type for resource kubernetes_persistent_volume (k8s api).

Fixes #1160

Acceptance tests

Have you added an acceptance test for the functionality being added?
Have you run the acceptance tests on this branch?

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccKubernetesPersistentVolume_azure_file'
...
TF_CLI_CONFIG_FILE=/tmp/terraform-provider-kubernetes/kubernetes/test-infra/external-providers/.terraformrc TF_PLUGIN_CACHE_DIR=/tmp/terraform-provider-kubernetes/kubernetes/test-infra/external-providers/.terraform TF_ACC=1 go test "/tmp/terraform-provider-kubernetes/kubernetes" -v -run=TestAccKubernetesPersistentVolume_azure_file -timeout 120m
=== RUN   TestAccKubernetesPersistentVolume_azure_file
--- PASS: TestAccKubernetesPersistentVolume_azure_file (321.06s)
PASS
ok      github.com/hashicorp/terraform-provider-kubernetes/kubernetes   321.600s

Output from kubectl during the test:

$ kubectl get pv/tfacctestbcvvfyt8dl -o yaml
apiVersion: v1
kind: PersistentVolume
metadata:
  name: tfacctestbcvvfyt8dl
spec:
  accessModes:
  - ReadWriteOnce
  azureFile:
    secretName: tfacctest8lkacq8lgx
    secretNamespace: tfacctestznnik4h68w
    shareName: tfacctestbcvvfyt8dl
  capacity:
    storage: 1Gi
  persistentVolumeReclaimPolicy: Retain
  volumeMode: Filesystem

Release Note

Release note for CHANGELOG:

Add support for updating `kubernetes_persistent_volume` `azure_file` `secret_namespace` argument.

References

Fixes #1160

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

hashicorp-cla · 2021-03-17T21:50:39Z

All committers have signed the CLA.

BasBerk · 2021-03-19T11:45:22Z

Hi,

Maybe my testing is wrong, I'm not an expert on these kind of things, but even using you code it still tries to look up the secret in the default namespace:

Warning FailedMount 30s (x2 over 32s) kubelet MountVolume.SetUp failed for volume "g7xxx" : Couldn't get secret default/common-sa-secret-dev

    volume {
       name = "g7xxx"
        azure_file {
          share_name = "data/g7_xxx"            
          secret_name = kubernetes_secret.common_sa_secret.metadata[0].name
          secret_namespace = local.namespace
          read_only = false
        }               
      }

+ volume { + name = "g7xxx" + azure_file { + read_only = false + secret_name = "common-sa-secret-dev" + secret_namespace = "iss-ftp-nonp-dev" + share_name = "data/g7_xxx" } }

bancek · 2021-03-19T12:08:27Z

@BazzardTCR it looks like I made a typo in my PR branch. I'll look into adding an acceptance test.

BasBerk · 2021-03-19T12:47:50Z

@BazzardTCR it looks like I made a typo in my PR branch. I'll look into adding an acceptance test.

I did a rebuild check if the last change was in there, it is, but still the same issue.
Not sure it helps or it's relevant, with kubectl describe pod:

shared: Type: AzureFile (an Azure File Service mount on the host and bind mount to the pod) SecretName: common-sa-secret-dev ShareName: shared ReadOnly: true

bancek · 2021-03-19T14:38:04Z

I've added the acceptance test and hardened the checks for nil and empty values. Changing secret_namespace now also forces recreate.

dak1n1

This PR looks really good! Thanks for this contribution. I'm going to run a few more tests, but I wanted to leave some initial feedback.

kubernetes/schema_volume_source.go

kubernetes/resource_kubernetes_persistent_volume_test.go

dak1n1 · 2021-03-29T20:07:49Z

kubernetes/resource_kubernetes_persistent_volume_test.go

+resource "kubernetes_secret" "test" {
+  metadata {
+    name      = %[3]q
+    namespace = %[2]q
+  }
+
+  data = {
+    azurestorageaccountname = azurerm_storage_account.test.name
+    azurestorageaccountkey  = azurerm_storage_account.test.primary_access_key
+  }
+}


These tests look great! I'm especially excited to see an example demonstrating a feature I haven't used before. Thanks for this!

Ideally we would also create a pod and validate that the volume is actually mounted but this is better than nothing :)

Good idea! I ended up doing that manually to test. I'll paste it here in case anyone is randomly googling how to mount azure file shares to their pod.

https://gist.githubusercontent.com/dak1n1/43a199ce6b066d894f5a878f3f588561/raw/d5f86fa57f4dd79bda4d93c1364897a180b9b8a1/gistfile1.txt

Co-authored-by: Stef Forrester <[email protected]>

dak1n1

I tested this on AKS 1.18, 1.19, 1.20 using terraform 0.14.7 and 0.14.8. It looks like it's working reliably. Thanks again!

dak1n1 · 2021-03-29T23:50:42Z

kubernetes/resource_kubernetes_persistent_volume_test.go

+resource "kubernetes_secret" "test" {
+  metadata {
+    name      = %[3]q
+    namespace = %[2]q
+  }
+
+  data = {
+    azurestorageaccountname = azurerm_storage_account.test.name
+    azurestorageaccountkey  = azurerm_storage_account.test.primary_access_key
+  }
+}


Good idea! I ended up doing that manually to test. I'll paste it here in case anyone is randomly googling how to mount azure file shares to their pod.

https://gist.githubusercontent.com/dak1n1/43a199ce6b066d894f5a878f3f588561/raw/d5f86fa57f4dd79bda4d93c1364897a180b9b8a1/gistfile1.txt

ghost · 2021-04-29T05:10:06Z

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 👉 [email protected]. Thanks!

ghost added size/XS labels Mar 17, 2021

ghost added documentation labels Mar 17, 2021

bancek mentioned this pull request Mar 17, 2021

kubernetes_persistent_volume to support arg secret_namespace for azure_file volume type #1160

Closed

Add secret_namespace to volume_source azure_file

84feb33

bancek force-pushed the feature-azure-file-secret-namespace branch from ac7c1c7 to 84feb33 Compare March 19, 2021 14:34

ghost added size/L and removed size/XS labels Mar 19, 2021

Base automatically changed from master to main March 23, 2021 15:53

dak1n1 added the acknowledged Issue has undergone initial review and is in our work queue. label Mar 29, 2021

dak1n1 reviewed Mar 29, 2021

View reviewed changes

dak1n1 self-assigned this Mar 29, 2021

Update kubernetes/schema_volume_source.go

632844e

Co-authored-by: Stef Forrester <[email protected]>

dak1n1 approved these changes Mar 29, 2021

View reviewed changes

dak1n1 merged commit 1039c8f into hashicorp:main Mar 29, 2021

ghost locked as resolved and limited conversation to collaborators Apr 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add secret_namespace to volume_source azure_file #1204

Add secret_namespace to volume_source azure_file #1204

bancek commented Mar 17, 2021 •

edited

Loading

hashicorp-cla commented Mar 17, 2021 •

edited

Loading

BasBerk commented Mar 19, 2021

bancek commented Mar 19, 2021

BasBerk commented Mar 19, 2021

bancek commented Mar 19, 2021

dak1n1 left a comment

dak1n1 Mar 29, 2021

bancek Mar 29, 2021

dak1n1 Mar 29, 2021

dak1n1 left a comment

dak1n1 Mar 29, 2021

ghost commented Apr 29, 2021

Add secret_namespace to volume_source azure_file #1204

Add secret_namespace to volume_source azure_file #1204

Conversation

bancek commented Mar 17, 2021 • edited Loading

Description

Acceptance tests

Release Note

References

Community Note

hashicorp-cla commented Mar 17, 2021 • edited Loading

BasBerk commented Mar 19, 2021

bancek commented Mar 19, 2021

BasBerk commented Mar 19, 2021

bancek commented Mar 19, 2021

dak1n1 left a comment

Choose a reason for hiding this comment

dak1n1 Mar 29, 2021

Choose a reason for hiding this comment

bancek Mar 29, 2021

Choose a reason for hiding this comment

dak1n1 Mar 29, 2021

Choose a reason for hiding this comment

dak1n1 left a comment

Choose a reason for hiding this comment

dak1n1 Mar 29, 2021

Choose a reason for hiding this comment

ghost commented Apr 29, 2021

bancek commented Mar 17, 2021 •

edited

Loading

hashicorp-cla commented Mar 17, 2021 •

edited

Loading