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.

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

r/pod+rc: Avoid crash in reading container.security_context capability #53

Merged
merged 1 commit into from
Aug 22, 2017

Conversation

radeksimko
Copy link
Member

Test results

make testacc TEST=./kubernetes TESTARGS='-run=TestAccKubernetesPod'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes -v -run=TestAccKubernetesPod -timeout 120m
=== RUN   TestAccKubernetesPod_basic
--- PASS: TestAccKubernetesPod_basic (24.63s)
=== RUN   TestAccKubernetesPod_updateForceNew
--- PASS: TestAccKubernetesPod_updateForceNew (103.62s)
=== RUN   TestAccKubernetesPod_importBasic
--- PASS: TestAccKubernetesPod_importBasic (7.68s)
=== RUN   TestAccKubernetesPod_with_pod_security_context
--- PASS: TestAccKubernetesPod_with_pod_security_context (4.74s)
=== RUN   TestAccKubernetesPod_with_container_liveness_probe_using_exec
--- PASS: TestAccKubernetesPod_with_container_liveness_probe_using_exec (50.10s)
=== RUN   TestAccKubernetesPod_with_container_liveness_probe_using_http_get
--- PASS: TestAccKubernetesPod_with_container_liveness_probe_using_http_get (5.51s)
=== RUN   TestAccKubernetesPod_with_container_liveness_probe_using_tcp
--- PASS: TestAccKubernetesPod_with_container_liveness_probe_using_tcp (6.22s)
=== RUN   TestAccKubernetesPod_with_container_lifecycle
--- PASS: TestAccKubernetesPod_with_container_lifecycle (7.67s)
=== RUN   TestAccKubernetesPod_with_container_security_context
--- PASS: TestAccKubernetesPod_with_container_security_context (4.72s)
=== RUN   TestAccKubernetesPod_with_volume_mount
--- PASS: TestAccKubernetesPod_with_volume_mount (20.90s)
=== RUN   TestAccKubernetesPod_with_cfg_map_volume_mount
--- PASS: TestAccKubernetesPod_with_cfg_map_volume_mount (6.81s)
=== RUN   TestAccKubernetesPod_with_resource_requirements
--- PASS: TestAccKubernetesPod_with_resource_requirements (6.80s)
=== RUN   TestAccKubernetesPod_with_empty_dir_volume
--- PASS: TestAccKubernetesPod_with_empty_dir_volume (11.94s)
=== RUN   TestAccKubernetesPod_with_secret_vol_items
--- PASS: TestAccKubernetesPod_with_secret_vol_items (10.18s)
=== RUN   TestAccKubernetesPod_with_nodeSelector
--- PASS: TestAccKubernetesPod_with_nodeSelector (11.99s)
PASS
ok  	github.com/terraform-providers/terraform-provider-kubernetes/kubernetes	283.576s

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

one minor question - otherwise LGTM :)

@@ -8,7 +8,7 @@ import (
)

func flattenCapability(in []v1.Capability) []string {
att := make([]string, 0, len(in))
att := make([]string, len(in), len(in))
Copy link
Contributor

Choose a reason for hiding this comment

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

I may be wrong, but I don't think this is needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is needed, because direct access via [i] requires elements to be already pre-allocated.

It wouldn't be necessary if we used append(), but that's unnecessary since we know precise length of the array. I would only use append when I don't know how many elements I'm going to have as it allocates and re-allocates the memory for you (i.e. possibly more than you really need).

https://play.golang.org/p/s9YliIhbQy
https://blog.golang.org/go-slices-usage-and-internals

@radeksimko radeksimko merged commit 15397f2 into master Aug 22, 2017
@radeksimko radeksimko deleted the b-container-sc-caps-crash branch August 22, 2017 05:10
@ghost ghost locked and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants