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

Per container ingress network override #480

Merged
merged 8 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ For containers, that would be the container IPs.

Only containers/services that are connected to Caddy ingress networks are used.

:warning: caddy docker proxy does a best effort to automatically detect what are the ingress networks. But that logic fails on some scenarios: [#207](https://github.com/lucaslorentz/caddy-docker-proxy/issues/207). To have a more resilient solution, you can manually configure Caddy ingress network using CLI option `ingress-networks` or environment variable `CADDY_INGRESS_NETWORKS`.
:warning: caddy docker proxy does a best effort to automatically detect what are the ingress networks. But that logic fails on some scenarios: [#207](https://github.com/lucaslorentz/caddy-docker-proxy/issues/207). To have a more resilient solution, you can manually configure Caddy ingress network using CLI option `ingress-networks`, environment variable `CADDY_INGRESS_NETWORKS` or label `caddy_ingress_network`.

Usage: `upstreams [http|https] [port]`

Expand Down
14 changes: 12 additions & 2 deletions generator/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,18 @@ func (g *CaddyfileGenerator) getContainerCaddyfile(container *types.Container, l
func (g *CaddyfileGenerator) getContainerIPAddresses(container *types.Container, logger *zap.Logger, ingress bool) ([]string, error) {
ips := []string{}

for _, network := range container.NetworkSettings.Networks {
if !ingress || g.ingressNetworks[network.NetworkID] {
ingressNetworkFromLabel, overrideNetwork := container.Labels[IngressNetworkLabel]

for networkName, network := range container.NetworkSettings.Networks {
include := false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There might be a simpler way to express this logic, but I believe the caddy_ingress_network label needs to be a complete override of the allowed networks. Otherwise, we could still return IPs from another network?

Copy link
Owner

@lucaslorentz lucaslorentz Apr 26, 2023

Choose a reason for hiding this comment

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

Yeah, that makes sense, can you please let param ingress overrule everything else? Basically, there is a place that calls this function to get all IPs, not only IPs for ingress.

Maybe doing an if !ingress before if overrideNetwork

Feel free to rename the ingress param as well to something more clear, maybe forIngress or onlyIngressIps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will go with onlyIngressIps


if overrideNetwork {
include = networkName == ingressNetworkFromLabel
} else {
include = !ingress || g.ingressNetworks[network.NetworkID]
}

if include {
ips = append(ips, network.IPAddress)
}
}
Expand Down
46 changes: 46 additions & 0 deletions generator/containers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,52 @@ func TestContainers_ManualIngressNetworks(t *testing.T) {
}, expectedCaddyfile, expectedLogs)
}

func TestContainers_OverrideIngressNetworks(t *testing.T) {
dockerClient := createBasicDockerClientMock()
dockerClient.NetworksData = []types.NetworkResource{
{
ID: "other-network-id",
Name: "other-network-name",
},
{
ID: "another-network-id",
Name: "another-network-name",
},
}
dockerClient.ContainersData = []types.Container{
{
ID: "CONTAINER-ID",
NetworkSettings: &types.SummaryNetworkSettings{
Networks: map[string]*network.EndpointSettings{
"other-network": {
IPAddress: "10.0.0.1",
NetworkID: "other-network-id",
},
"another-network": {
IPAddress: "10.0.0.2",
NetworkID: "other-network-id",
},
},
},
Labels: map[string]string{
"caddy_ingress_network": "another-network",
fmtLabel("%s"): "service.testdomain.com",
fmtLabel("%s.reverse_proxy"): "{{upstreams}}",
},
},
}

const expectedCaddyfile = "service.testdomain.com {\n" +
" reverse_proxy 10.0.0.2\n" +
"}\n"

const expectedLogs = otherIngressNetworksMapLog + swarmIsAvailableLog

testGeneration(t, dockerClient, func(options *config.Options) {
options.IngressNetworks = []string{"other-network-name"}
}, expectedCaddyfile, expectedLogs)
}

func TestContainers_Replicas(t *testing.T) {
dockerClient := createBasicDockerClientMock()
dockerClient.ContainersData = []types.Container{
Expand Down
2 changes: 2 additions & 0 deletions generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
// DefaultLabelPrefix for caddy labels in docker
const DefaultLabelPrefix = "caddy"

const IngressNetworkLabel = "caddy_ingress_network"

const swarmAvailabilityCacheInterval = 1 * time.Minute

// CaddyfileGenerator generates caddyfile from docker configuration
Expand Down
12 changes: 11 additions & 1 deletion generator/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,18 @@ func (g *CaddyfileGenerator) getServiceTasksIps(service *swarm.Service, logger *
for _, task := range tasks {
if task.Status.State == swarm.TaskStateRunning {
hasRunningTasks = true
ingressNetworkFromLabel, overrideNetwork := service.Spec.Labels[IngressNetworkLabel]

for _, networkAttachment := range task.NetworksAttachments {
if !ingress || g.ingressNetworks[networkAttachment.Network.ID] {
include := false

if overrideNetwork {
include = networkAttachment.Network.Spec.Name == ingressNetworkFromLabel
} else {
include = !ingress || g.ingressNetworks[networkAttachment.Network.ID]
}

if include {
for _, address := range networkAttachment.Addresses {
ipAddress, _, _ := net.ParseCIDR(address)
tasksIps = append(tasksIps, ipAddress.String())
Expand Down
46 changes: 46 additions & 0 deletions generator/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,52 @@ func TestServices_ManualIngressNetwork(t *testing.T) {
}, expectedCaddyfile, expectedLogs)
}

func TestServices_OverrideIngressNetwork(t *testing.T) {
dockerClient := createBasicDockerClientMock()
dockerClient.NetworksData = []types.NetworkResource{
{
ID: "other-network-id",
Name: "other-network-name",
},
{
ID: "another-network-id",
Name: "another-network-name",
},
}
dockerClient.ServicesData = []swarm.Service{
{
ID: "SERVICE-ID",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{
Name: "service",
Labels: map[string]string{
"caddy_ingress_network": "another-network",
fmtLabel("%s"): "service.testdomain.com",
fmtLabel("%s.reverse_proxy"): "{{upstreams}}",
},
},
},
Endpoint: swarm.Endpoint{
VirtualIPs: []swarm.EndpointVirtualIP{
{
NetworkID: "other-network-id",
},
},
},
},
}

const expectedCaddyfile = "service.testdomain.com {\n" +
" reverse_proxy service\n" +
"}\n"

const expectedLogs = otherIngressNetworksMapLog + swarmIsAvailableLog

testGeneration(t, dockerClient, func(options *config.Options) {
options.IngressNetworks = []string{"other-network-name"}
Copy link
Owner

@lucaslorentz lucaslorentz Apr 26, 2023

Choose a reason for hiding this comment

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

You're right, this test is not meaningful as it is right now.
But swarm service proxying supports 2 modes, proxying to virutal service using its dns name, or proxying to all service tasks (containers) IPs.
Add another option after this line like:

options.ProxyServiceTasks = true

This will swap the behavior to IPs, and then adjust the expectedCaddyfile to match the IP addresses configured in dockerClient.ServicesData

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. If I understand correctly, I should use TestServiceTasks_ManualIngressNetwork as a reference. Is overriding the ingress network to a virtual service something that should be tested?

Copy link
Owner

Choose a reason for hiding this comment

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

No need

}, expectedCaddyfile, expectedLogs)
}

func TestServices_SwarmDisabled(t *testing.T) {
dockerClient := createBasicDockerClientMock()
dockerClient.ServicesData = []swarm.Service{
Expand Down
4 changes: 2 additions & 2 deletions tests/functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

function retry {
local n=0
local max=5
local delay=20
local max=20
local delay=5
while true; do
((n=n+1))
"$@" && break || {
Expand Down
19 changes: 19 additions & 0 deletions tests/ingress-networks/compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ services:
- controller
- ingress_0
- ingress_1
- ingress_2
environment:
- CADDY_DOCKER_MODE=server
- CADDY_CONTROLLER_NETWORK=10.200.200.0/24
Expand Down Expand Up @@ -54,11 +55,29 @@ services:
caddy.reverse_proxy: "{{upstreams 80}}"
caddy.tls: "internal"

# Proxy to service
whoami2:
image: containous/whoami
networks:
- internal
- ingress_2
deploy:
labels:
caddy: whoami2.example.com
caddy.reverse_proxy: "{{upstreams 80}}"
caddy.tls: "internal"
caddy_ingress_network: ingress_2

networks:
ingress_0:
name: ingress_0
ingress_1:
name: ingress_1
ingress_2:
name: ingress_2
internal:
name: internal
internal: true
controller:
driver: overlay
ipam:
Expand Down
3 changes: 2 additions & 1 deletion tests/ingress-networks/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ set -e
docker stack deploy -c compose.yaml --prune caddy_test

retry curl --show-error -s -k -f --resolve whoami0.example.com:443:127.0.0.1 https://whoami0.example.com &&
retry curl --show-error -s -k -f --resolve whoami1.example.com:443:127.0.0.1 https://whoami1.example.com || {
retry curl --show-error -s -k -f --resolve whoami1.example.com:443:127.0.0.1 https://whoami1.example.com &&
retry curl --show-error -s -k -f --resolve whoami2.example.com:443:127.0.0.1 https://whoami2.example.com || {
docker service logs caddy_test_caddy_controller
docker service logs caddy_test_caddy_server
exit 1
Expand Down
17 changes: 16 additions & 1 deletion tests/standalone/compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ services:
caddy.reverse_proxy: "{{upstreams 80}}"
caddy.tls: "internal"

# Proxy to container
whoami4:
image: containous/whoami
networks:
- internal
- caddy
labels:
caddy: whoami4.example.com
caddy.reverse_proxy: "{{upstreams 80}}"
caddy.tls: "internal"
caddy_ingress_network: caddy_test

# Proxy with matches and route
echo_0:
image: containous/whoami
Expand All @@ -74,4 +86,7 @@ services:
networks:
caddy:
name: caddy_test
external: true
external: true
internal:
name: internal
internal: true
1 change: 1 addition & 0 deletions tests/standalone/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ retry curl --show-error -s -k -f --resolve whoami0.example.com:443:127.0.0.1 htt
retry curl --show-error -s -k -f --resolve whoami1.example.com:443:127.0.0.1 https://whoami1.example.com &&
retry curl --show-error -s -k -f --resolve whoami2.example.com:443:127.0.0.1 https://whoami2.example.com &&
retry curl --show-error -s -k -f --resolve whoami3.example.com:443:127.0.0.1 https://whoami3.example.com &&
retry curl --show-error -s -k -f --resolve whoami4.example.com:443:127.0.0.1 https://whoami4.example.com &&
retry curl --show-error -s -k -f --resolve echo0.example.com:443:127.0.0.1 https://echo0.example.com/sourcepath/something || {
docker service logs caddy_test_caddy
exit 1
Expand Down