Add support for importing service accounts #377
Conversation
This implements importing of service accounts using the behaviour implemented in source code of the Kubernetes service account controller to discover the default service account token. This PR also adds support for updating the `automount_service_account_token` attribute when it changes.
|
This would fix #268 |
|
|
||
| namespace, name, err := idParts(d.Id()) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Would you mind wrapping these errors before returning them? The idea being to make it obvious that the error happened during import processing, since these can also occur during a normal Read.
| return "", fmt.Errorf("Unable to find any service accounts tokens which could have been the default one") | ||
| } | ||
|
|
||
| if len(serviceAccountTokens) > 1 { |
There was a problem hiding this comment.
If the goal is to find exactly one, then finding the first ones means it's done.
You could just return it when you find it rather than running the list of secrets all the way to the end.
There was a problem hiding this comment.
The thought was to be as conservative as possible and just fail if the assumptions used to find the default service account aren't correct
There was a problem hiding this comment.
I'll refactor it so that is less conservative
| @@ -169,6 +169,10 @@ func resourceKubernetesServiceAccountRead(d *schema.ResourceData, meta interface | |||
| if err != nil { | |||
| return err | |||
| } | |||
| err = d.Set("automount_service_account_token", *svcAcc.AutomountServiceAccountToken) | |||
There was a problem hiding this comment.
This crashes with a panic if svcAcc.AutomountServiceAccountToken is nil.
It should be checked for nil before using it, as with all pointer attributes.
There was a problem hiding this comment.
This looks good overall.
I've left some comments below on spot issues.
On top of that, could you please remove the default value from automount_service_account_token attribute. It's not part of the Kubernetes spec and it also causes a diff after importing a new which doesn't have it set.
|
Thanks for the updates @wjam. |
|
FYI, I've just tested the importation of four service accounts building from master and this works perfectly, thanks! |
|
Also, this will be worth mentioning in the release notes as I thought this was already supported in the last release:
|
|
It doesn't work (reliably) for me because Secret is sometimes created >1 seconds after ServiceAccount. In my case: The outcome of "terraform import kubernetes_service_account.kube-system_aws-node kube-system/aws-node" is: |
Importing service accounts isn't always working on EKS clusters, which sometimes see secrets created more than one second after the service account, as reported in hashicorp#377 (comment) and hashicorp#268 (comment) With some fields removed for clarity: ``` { "kind": "ServiceAccount", "metadata": { "name": "coredns", "uid": "e2885307-37d7-11ea-9db3-1211528e452b", "resourceVersion": "201", "creationTimestamp": "2020-01-15T20:44:54Z", }, "secrets": [ { "name": "coredns-token-8tdpj" } ] } { "kind": "Secret", "metadata": { "name": "coredns-token-8tdpj", "resourceVersion": "196", "creationTimestamp": "2020-01-15T20:44:56Z", "annotations": { "kubernetes.io/service-account.name": "coredns", "kubernetes.io/service-account.uid": "e2885307-37d7-11ea-9db3-1211528e452b" } }, "type": "kubernetes.io/service-account-token" } ``` It's not clear what could be causing this. In our case, the cluster was brand new at the time the account was created, as can be seen in the relatively low resourceVersions. Maybe it's load, maybe it's clock drift between API servers (where creationTimestamp is injected, AFAIK). No matter the cause, this is a real problem and it's stopping imports. This is the simplest fix. A more comprehensive one could also double check that the annotations for SA name and the UID on the secret match with the account's. If more than one secret matches all criteria, perhaps the oldest one could be picked. But that's all better addressed separately.
Importing service accounts isn't always working on EKS clusters, which sometimes see secrets created more than one second after the service account, as reported in #377 (comment) and #268 (comment) With some fields removed for clarity: ``` { "kind": "ServiceAccount", "metadata": { "name": "coredns", "uid": "e2885307-37d7-11ea-9db3-1211528e452b", "resourceVersion": "201", "creationTimestamp": "2020-01-15T20:44:54Z", }, "secrets": [ { "name": "coredns-token-8tdpj" } ] } { "kind": "Secret", "metadata": { "name": "coredns-token-8tdpj", "resourceVersion": "196", "creationTimestamp": "2020-01-15T20:44:56Z", "annotations": { "kubernetes.io/service-account.name": "coredns", "kubernetes.io/service-account.uid": "e2885307-37d7-11ea-9db3-1211528e452b" } }, "type": "kubernetes.io/service-account-token" } ``` It's not clear what could be causing this. In our case, the cluster was brand new at the time the account was created, as can be seen in the relatively low resourceVersions. Maybe it's load, maybe it's clock drift between API servers (where creationTimestamp is injected, AFAIK). No matter the cause, this is a real problem and it's stopping imports. This is the simplest fix. A more comprehensive one could also double check that the annotations for SA name and the UID on the secret match with the account's. If more than one secret matches all criteria, perhaps the oldest one could be picked. But that's all better addressed separately.
ghost
locked and limited conversation to collaborators
This implements importing of service accounts using the behaviour implemented in source code of the Kubernetes service account controller to discover the default service account token. This PR also adds support for updating the
automount_service_account_tokenattribute when it changes.