Remove default token volume and volume_mount from Pod state

[dak1n1]

Fixes #1085

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:

Release Note

Release note for CHANGELOG:

Ensure default service account token is not saved in Pod state.

References

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

[jrhouston]

The reason this isn't working is because your removeVolume func returns a new slice, so you need to assign that new slice to in.Volumes.

[jrhouston]

What I would do is put this logic here in the loop that expands the volumes and use a continue statement to skip it, rather than removing it here.

edit: we'll have to change that code to use append rather than creating a fixed size array

[dak1n1]

Hmm, well this does look neater. And it successfully skips adding both the volume and volumeMount to state... well, almost. It adds them as empty blocks if I put the logic into flattenVolumes() and flattenContainerVolumeMounts(), rather than in flattenPodSpec() . ("Empty blocks" meaning: volume{} and volume_mount{}). Here is the state for context:

https://gist.githubusercontent.com/dak1n1/3ec5fc36cb9c4ab16d7d5ed8c159009c/raw/bc670e3486d17d5eb7eafe8016ebb1e63d09d06f/gistfile1.txt

However, if I remove these at the PodSpec level, it omits the blocks entirely, which prevents the diff. I'm going to explore that route next.

[dak1n1]

Got it working! Pods are no longer destroyed on each apply.

[dakini@dax pod_with_volume]$ terraform show 
[...]
            volume_mount {
                mount_path        = "/etc/test"
                mount_propagation = "None"
                name              = "pvc"
                read_only         = false
            }
        }

        volume {
            name = "pvc"

            persistent_volume_claim {
                claim_name = "test"
                read_only  = false
            }
        }
    }
}
[dakini@dax pod_with_volume]$ terraform plan
kubernetes_persistent_volume_claim.test: Refreshing state... [id=default/test]
kubernetes_pod.main: Refreshing state... [id=default/test]

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.

[jrhouston]

Ah, I just realized we are going to have to be a bit more clever than this.

When the token get's auto-mounted it takes the name from the service account, so only if they don't supply a service account will it be called default, otherwise it will be the name of the service account. You can see how it does this in the admission controller code for it: kubernetes/blob/master/plugin/pkg/admission/serviceaccount/admission.go#L46

So we'll have to actually pass down the serviceAccount name and make the regex match that. 😢

We could just match on -token- but that creates a surprising edge case for someone to discover.

[dak1n1]

oh, nice! That is really helpful. I hadn't thought to test with a service account specified. I'll put in a test to catch these cases and make it clever enough to handle them. Thanks!

[dak1n1]

It works now. I'll put the test into PR #1074, since that contains the test for bug #1085

[jrhouston]

Looks good just a couple of nits.

[jrhouston]

🚀

[ghost]

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!