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

tproxy: networking hook changes #20183

Merged
merged 6 commits into from
Mar 27, 2024
Merged

tproxy: networking hook changes #20183

merged 6 commits into from
Mar 27, 2024

Conversation

tgross
Copy link
Member

@tgross tgross commented Mar 21, 2024

When transparent_proxy block is present and the network mode is bridge, use a different CNI configuration that includes the consul-cni plugin. Before invoking the CNI plugins, create a Consul SDK iptables.Config struct for the allocation. This includes:

  • Use all the transparent_proxy block fields
  • The reserved ports are added to the inbound exclusion list so the alloc is reachable from outside the mesh
  • The expose blocks and check blocks with expose=true are added to the inbound exclusion list so health checks work.

The iptables.Config is then passed as a CNI argument to the consul-cni plugin.

Ref: #10628
Ref: hashicorp/consul-k8s#3795


This PR targets the feature branch. In addition to the new unit tests, I've verified the behavior of this PR with a build of consul-cni from hashicorp/consul-k8s#3795 and the following jobspec which is our usual "countdash" Connect example, but with transparent proxy and health checking added:

countdash with tproxy
job "countdash" {

  group "api" {
    network {
      mode = "bridge"
    }

    service {
      name = "count-api"
      port = "9001"

      check {
        type     = "http"
        path     = "/health"
        expose   = true
        interval = "3s"
        timeout  = "1s"

        check_restart {
          limit = 0
        }
      }

      connect {
        sidecar_service {
          proxy {
            transparent_proxy {}
          }
        }
      }
    }

    task "web" {
      driver = "docker"

      config {
        image          = "hashicorpdev/counter-api:v3"
        auth_soft_fail = true
      }
    }
  }

  group "dashboard" {
    network {
      mode = "bridge"

      port "http" {
        static = 9010
        to     = 9002
      }
    }

    service {
      name = "count-dashboard"
      port = "9002"

      check {
        type     = "http"
        path     = "/health"
        expose   = true
        interval = "3s"
        timeout  = "1s"

        check_restart {
          limit = 0
        }
      }

      connect {
        sidecar_service {
          proxy {
            transparent_proxy {}
          }
        }
      }
    }

    task "dashboard" {
      driver = "docker"

      env {
        COUNTING_SERVICE_URL = "http://count-api.virtual.consul:9001"
      }

      config {
        image          = "hashicorpdev/counter-dashboard:v3"
        auth_soft_fail = true
      }
    }
  }
}

@tgross tgross added this to the 1.8.0 milestone Mar 21, 2024
@tgross tgross mentioned this pull request Mar 21, 2024
10 tasks
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
@tgross tgross requested review from gulducat and shoenig March 26, 2024 18:08
client/client.go Outdated Show resolved Hide resolved
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM!!

@tgross
Copy link
Member Author

tgross commented Mar 27, 2024

Holding off merging this until this conversation is resolved: https://github.com/hashicorp/consul-k8s/pull/3795/files#r1539851537. If it takes a while to get resolved I might merge as-is anyways and then just push any change needed to the arg name to the feature branch as a separate commit. Resolved in 6f63b81

Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

looks good! just a few tidbits for your consideration

client/allocrunner/networking_bridge_linux.go Outdated Show resolved Hide resolved
client/allocrunner/networking_cni.go Show resolved Hide resolved
client/allocrunner/networking_cni.go Outdated Show resolved Hide resolved
client/allocrunner/networking_cni.go Outdated Show resolved Hide resolved
client/allocrunner/networking_cni.go Show resolved Hide resolved
client/allocrunner/networking_cni.go Outdated Show resolved Hide resolved
@tgross tgross merged commit 2045e62 into f-tproxy Mar 27, 2024
18 checks passed
@tgross tgross deleted the f-tproxy-network-config branch March 27, 2024 18:24
tgross added a commit that referenced this pull request Mar 27, 2024
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
tgross added a commit that referenced this pull request Mar 27, 2024
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
tgross added a commit that referenced this pull request Mar 29, 2024
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
tgross added a commit that referenced this pull request Apr 3, 2024
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
tgross added a commit that referenced this pull request Apr 4, 2024
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
tgross added a commit that referenced this pull request Apr 4, 2024
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
philrenaud pushed a commit that referenced this pull request Apr 18, 2024
When `transparent_proxy` block is present and the network mode is `bridge`, use
a different CNI configuration that includes the `consul-cni` plugin. Before
invoking the CNI plugins, create a Consul SDK `iptables.Config` struct for the
allocation. This includes:

* Use all the `transparent_proxy` block fields
* The reserved ports are added to the inbound exclusion list so the alloc is
  reachable from outside the mesh
* The `expose` blocks and `check` blocks with `expose=true` are added to the
  inbound exclusion list so health checks work.

The `iptables.Config` is then passed as a CNI argument to the `consul-cni`
plugin.

Ref: #10628
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants