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

Add network policy resource with support for 1.8+ fields #118

Merged
merged 17 commits into from
Jan 10, 2019

Conversation

pdecat
Copy link
Contributor

@pdecat pdecat commented Feb 14, 2018

This is basically the same as #113 but with support for k8s 1.8+ Network Policy fields: egress, ip_block and policy_types.

This requires #117 for up to date k8s 1.9 client libraries.
Up to date k8s 1.10 client libraries provided by #162

TODO:

  • Rebase on master
  • Fix egress with single empty map on first apply
  • Add documentation for the new fields

@pdecat pdecat force-pushed the f-network-policy-support-1.9 branch from 7419cc5 to 405e938 Compare February 14, 2018 01:49
@pdecat
Copy link
Contributor Author

pdecat commented Feb 14, 2018

Unit tests results

# make test                                                                                    
==> Checking that code complies with gofmt requirements...        
go test -i $(go list ./... |grep -v 'vendor') || exit 1           
echo $(go list ./... |grep -v 'vendor') | \
        xargs -t -n4 go test  -timeout=30s -parallel=4
go test -timeout=30s -parallel=4 github.com/terraform-providers/terraform-provider-kubernetes github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 
?       github.com/terraform-providers/terraform-provider-kubernetes    [no test files]
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 0.027s

@pdecat
Copy link
Contributor Author

pdecat commented Feb 14, 2018

Acceptance tests results

# make testacc TEST=./kubernetes/ TESTARGS='-run=TestAccKubernetesNetworkPolicy_.*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes/ -v -run=TestAccKubernetesNetworkPolicy_.* -timeout 120m
=== RUN   TestAccKubernetesNetworkPolicy_basic
--- PASS: TestAccKubernetesNetworkPolicy_basic (0.47s)
=== RUN   TestAccKubernetesNetworkPolicy_importBasic
--- PASS: TestAccKubernetesNetworkPolicy_importBasic (0.12s)
PASS
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 0.606s

@pdecat
Copy link
Contributor Author

pdecat commented Mar 29, 2018

There is a bug that requires applying the configuration twice when egress contains a single empty map:

resource "kubernetes_network_policy" "allow-all-egress" {
  metadata {
    name = "allow-all-egress"
  }

  spec {
    pod_selector = {}

    ingress = [] # no rule at all to deny all ingress traffic

    egress = [{}] # single empty rule to allow all egress traffic
  }
}

After first apply, the following diff is still there:

  ~ kubernetes_network_policy.allow-all-egress                    
      spec.0.egress.#: "0" => "1"                                                                      

On second apply, it is gone.

@pdecat pdecat force-pushed the f-network-policy-support-1.9 branch from 773cec0 to 15e6210 Compare April 18, 2018 08:33
@pdecat pdecat force-pushed the f-network-policy-support-1.9 branch from 15e6210 to 0ba7866 Compare May 7, 2018 06:16
@pdecat
Copy link
Contributor Author

pdecat commented May 15, 2018

The issue when the Network Policy has an egress at creation is resolved.

@pdecat
Copy link
Contributor Author

pdecat commented May 15, 2018

Acceptance tests results

make testacc TEST=./kubernetes TESTARGS='-run=TestAccKubernetesNetworkPolicy_.*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes -v -run=TestAccKubernetesNetworkPolicy_.* -timeout 120m
=== RUN   TestAccKubernetesNetworkPolicy_basic
--- PASS: TestAccKubernetesNetworkPolicy_basic (0.40s)
=== RUN   TestAccKubernetesNetworkPolicy_withEgressAtCreation
--- PASS: TestAccKubernetesNetworkPolicy_withEgressAtCreation (0.11s)
=== RUN   TestAccKubernetesNetworkPolicy_importBasic
--- PASS: TestAccKubernetesNetworkPolicy_importBasic (0.11s)
PASS
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes

@pdecat
Copy link
Contributor Author

pdecat commented May 15, 2018

Acceptance tests results with policy_types assertions

make testacc TEST=./kubernetes TESTARGS='-run=TestAccKubernetesNetworkPolicy_.*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes -v -run=TestAccKubernetesNetworkPolicy_.* -timeout 120m
=== RUN   TestAccKubernetesNetworkPolicy_basic                                                                                                      
--- PASS: TestAccKubernetesNetworkPolicy_basic (0.40s)                         
=== RUN   TestAccKubernetesNetworkPolicy_withEgressAtCreation                                 
--- PASS: TestAccKubernetesNetworkPolicy_withEgressAtCreation (0.12s)                                                                               
=== RUN   TestAccKubernetesNetworkPolicy_importBasic
--- PASS: TestAccKubernetesNetworkPolicy_importBasic (0.10s)                                                                                                               
PASS                                                                                                                                                                     
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 0.647s                                         

@pdecat
Copy link
Contributor Author

pdecat commented May 15, 2018

Note

I've made the policy_types property required because the default value is only evaluated server side on resource creation.

During the initial creation, a default value is determined and stored, then PolicyTypes is no longer considered unset, it will stick to that value on further updates unless explicitly overridden.

Leaving the policy_types property optional here would prevent further updates adding egress rules after the initial resource creation without egress rules nor policy_types from working as expected as PolicyTypes will stick to Ingress server side.

Edit: I initially considered leaving it optional and overriding the default behavior client side but this started to make it overly complicated.

@pdecat
Copy link
Contributor Author

pdecat commented Oct 5, 2018

Rebased on master to use k8s 1.10 clients libraries.

@pdecat
Copy link
Contributor Author

pdecat commented Oct 5, 2018

Acceptance tests results

New run after rebase on master that provides k8s 1.10 client libs with local minikube 0.29/kubernetes 1.10.0:

# make testacc TEST=./kubernetes TESTARGS='-run=TestAccKubernetesNetworkPolicy_.*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes -v -run=TestAccKubernetesNetworkPolicy_.* -timeout 120m
=== RUN   TestAccKubernetesNetworkPolicy_basic
--- PASS: TestAccKubernetesNetworkPolicy_basic (0.55s)
=== RUN   TestAccKubernetesNetworkPolicy_withEgressAtCreation
--- PASS: TestAccKubernetesNetworkPolicy_withEgressAtCreation (0.13s)
=== RUN   TestAccKubernetesNetworkPolicy_importBasic
--- PASS: TestAccKubernetesNetworkPolicy_importBasic (0.11s)
PASS
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 0.839s

@pdecat
Copy link
Contributor Author

pdecat commented Oct 5, 2018

I've added the documentation for the k8s 1.8+ fields.

@pdecat
Copy link
Contributor Author

pdecat commented Oct 6, 2018

Acceptance tests results

New run with local minikube 0.30 / kubernetes 1.12.1:

# make testacc TEST=./kubernetes TESTARGS='-run=TestAccKubernetesNetworkPolicy_.*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes -v -run=TestAccKubernetesNetworkPolicy_.* -timeout 120m
=== RUN   TestAccKubernetesNetworkPolicy_basic
--- PASS: TestAccKubernetesNetworkPolicy_basic (0.48s)
=== RUN   TestAccKubernetesNetworkPolicy_withEgressAtCreation
--- PASS: TestAccKubernetesNetworkPolicy_withEgressAtCreation (0.15s)
=== RUN   TestAccKubernetesNetworkPolicy_importBasic
--- PASS: TestAccKubernetesNetworkPolicy_importBasic (0.11s)
PASS
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 0.786s

@pdecat pdecat force-pushed the f-network-policy-support-1.9 branch 2 times, most recently from cc2f6b6 to af19aa2 Compare October 18, 2018 07:19
@pdecat pdecat force-pushed the f-network-policy-support-1.9 branch from af19aa2 to 4736f9a Compare October 24, 2018 10:38
@pdecat pdecat force-pushed the f-network-policy-support-1.9 branch from 4736f9a to c00c61c Compare November 2, 2018 15:46
@pdecat
Copy link
Contributor Author

pdecat commented Nov 10, 2018

Hi @alexsomesan, would you mind having a look at this PR? I believe it would be a good candidate for the v1.4.0 milestone.

@pdecat
Copy link
Contributor Author

pdecat commented Jan 10, 2019

Rebased on master.

@pdecat pdecat force-pushed the f-network-policy-support-1.9 branch from 05de880 to 5c45144 Compare January 10, 2019 12:11
@pdecat
Copy link
Contributor Author

pdecat commented Jan 10, 2019

Acceptance tests results:

# make testacc TEST=./kubernetes TESTARGS='-run=TestAccKubernetesNetworkPolicy_.* -count=1'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes -v -run=TestAccKubernetesNetworkPolicy_.* -count=1 -timeout 120m
=== RUN   TestAccKubernetesNetworkPolicy_basic
--- PASS: TestAccKubernetesNetworkPolicy_basic (5.30s)
=== RUN   TestAccKubernetesNetworkPolicy_withEgressAtCreation
--- PASS: TestAccKubernetesNetworkPolicy_withEgressAtCreation (1.26s)
=== RUN   TestAccKubernetesNetworkPolicy_importBasic
--- PASS: TestAccKubernetesNetworkPolicy_importBasic (1.33s)
PASS
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 7.937s

Copy link
Member

@alexsomesan alexsomesan left a comment

Choose a reason for hiding this comment

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

This looks very nice overall.

Just one small thing: please remove the d.SetId("") in the Delete function, then we're good to go.

Did you test build this with Go 1.11.x?


// Use generated swagger docs from kubernetes' client-go to avoid copy/pasting them here
var (
networkPolicySpecDoc = api.NetworkPolicy{}.SwaggerDoc()["spec"]
Copy link
Member

Choose a reason for hiding this comment

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

Nice idea!

kubernetes/resource_kubernetes_network_policy.go Outdated Show resolved Hide resolved
@pdecat
Copy link
Contributor Author

pdecat commented Jan 10, 2019

All recent builds and tests done with:

# go version
go version go1.11.4 linux/amd64

@pdecat
Copy link
Contributor Author

pdecat commented Jan 10, 2019

Acceptance tests results:

# make testacc TEST=./kubernetes TESTARGS='-run=TestAccKubernetesNetworkPolicy_.* -count=1'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./kubernetes -v -run=TestAccKubernetesNetworkPolicy_.* -count=1 -timeout 120m
=== RUN   TestAccKubernetesNetworkPolicy_basic
--- PASS: TestAccKubernetesNetworkPolicy_basic (5.28s)
=== RUN   TestAccKubernetesNetworkPolicy_withEgressAtCreation
--- PASS: TestAccKubernetesNetworkPolicy_withEgressAtCreation (1.25s)
=== RUN   TestAccKubernetesNetworkPolicy_importBasic
--- PASS: TestAccKubernetesNetworkPolicy_importBasic (1.34s)
PASS
ok      github.com/terraform-providers/terraform-provider-kubernetes/kubernetes 7.912s

Copy link
Member

@alexsomesan alexsomesan left a comment

Choose a reason for hiding this comment

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

LGTM

@alexsomesan alexsomesan merged commit bd7821f into hashicorp:master Jan 10, 2019
@pdecat pdecat deleted the f-network-policy-support-1.9 branch January 10, 2019 13:43
@pdecat
Copy link
Contributor Author

pdecat commented Jan 10, 2019

Thank you so much @alexsomesan !

@@ -8,6 +8,10 @@ BUG FIXES:

* `resource/kubernetes_stateful_set`: Fix updates of stateful set images ([#252](https://github.com/terraform-providers/terraform-provider-kubernetes/issues/252))

FEATURES:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexsomesan I should probably have put FEATURES before IMPROVEMENTS and BUG FIXES

@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.

4 participants