Update Secret Volume to support items.
#44
Conversation
This works the same as `config_map.items`. I also fixed `validateAttributeValueDoesNotContain()` validator. It’s logic was the reverse of intention.
|
There was a problem hiding this comment.
Hi @sl1pm4t
thanks for this addition!
This is looking pretty good.
Besides comments I left in the code we'll also need to update documentation in the following places:
https://github.com/terraform-providers/terraform-provider-kubernetes/blob/master/website/docs/r/pod.html.markdown
https://github.com/terraform-providers/terraform-provider-kubernetes/blob/master/website/docs/r/replication_controller.html.markdown
The codepath is used by both of these two resources, AFAIK - correct me if I'm wrong.
kubernetes/schema_pod_spec.go
Outdated
| Type: schema.TypeInt, | ||
| Description: "Optional: mode bits to use on created files by default. Must be a value between 0 and 0777. Defaults to 0644. Directories within the path are not affected by this setting. This might be in conflict with other options that affect the file mode, like fsGroup, and the result can be other mode bits set.", | ||
| Optional: true, | ||
| ValidateFunc: validateModeBits, |
There was a problem hiding this comment.
Does it make sense to add Default: 0644 here? Either for clarity for the user when they see the diff and/or for consistency, otherwise the default value (if undefined) here will be 0 which I'm not sure is the same in this context.
kubernetes/structures_pod.go
Outdated
| for i, v := range in.Items { | ||
| m := map[string]interface{}{} | ||
| m["key"] = v.Key | ||
| m["mode"] = int(*v.Mode) |
There was a problem hiding this comment.
Should we add a nil check here in case v.Mode is nil to prevent crash?
kubernetes/structures_pod.go
Outdated
| } | ||
| att["items"] = items | ||
| } | ||
| att["optional"] = *in.Optional |
There was a problem hiding this comment.
We can't assume this codepath will only ever process resources created by Terraform. Keep in mind that it's also used for imports - so even if we always pass Optional to the API when creating the resource, it doesn't mean it will always come back.
TL;DR we should add a nil check, IMO.
| items { | ||
| key = "one" | ||
| path = "path/to/one" | ||
| } |
There was a problem hiding this comment.
Nitpick: do you mind fixing the indentation here?
kubernetes/schema_pod_spec.go
Outdated
| }, | ||
| "items": { | ||
| Type: schema.TypeList, | ||
| Description: `If unspecified, each key-value pair in the Data field of the referenced Secret will be projected into the volume as a file whose name is the key and content is the value. If specified, the listed keys will be projected into the specified paths, and unlisted keys will not be present. If a key is specified which is not present in the Secret, the volume setup will error. Paths must be relative and may not contain the '..' path or start with '..'.`, |
There was a problem hiding this comment.
Nitpick: Do you mind keeping one-line strings enclosed in double quotes? 🙂
sl1pm4t
force-pushed
the
volume-secret-item
branch
from
f84233c
to
97a6a63
Compare
|
Thank you for making those changes quickly. This LGTM now. 👍 |
ghost
locked and limited conversation to collaborators
This works the same as
config_map.items.I also fixed
validateAttributeValueDoesNotContain()validator. It’slogic was the reverse of intention.