Merge pull request #50506 from robmry/backport-28.x/fix_firewalld_reload

[28.x backport] Fix firewalld reload for per-endpoint rules

api/server/router/image/image_routes.go

@@ -100,6 +100,8 @@ func (ir *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrit
 
 		// For a pull it is not an error if no auth was given. Ignore invalid
 		// AuthConfig to increase compatibility with the existing API.
+		//
+		// TODO(thaJeztah): accept empty values but return an error when failing to decode.
 		authConfig, _ := registry.DecodeAuthConfig(r.Header.Get(registry.AuthHeader))
 		progressErr = ir.backend.PullImage(ctx, ref, platform, metaHeaders, authConfig, output)
 	} else { // import
@@ -167,16 +169,11 @@ func (ir *imageRouter) postImagesPush(ctx context.Context, w http.ResponseWriter
 
 	var authConfig *registry.AuthConfig
 	if authEncoded := r.Header.Get(registry.AuthHeader); authEncoded != "" {
-		// the new format is to handle the authConfig as a header. Ignore invalid
-		// AuthConfig to increase compatibility with the existing API.
+		// Handle the authConfig as a header, but ignore invalid AuthConfig
+		// to increase compatibility with the existing API.
+		//
+		// TODO(thaJeztah): accept empty values but return an error when failing to decode.
 		authConfig, _ = registry.DecodeAuthConfig(authEncoded)
-	} else {
-		// the old format is supported for compatibility if there was no authConfig header
-		var err error
-		authConfig, err = registry.DecodeAuthConfigBody(r.Body)
-		if err != nil {
-			return errors.Wrap(err, "bad parameters and missing X-Registry-Auth")
-		}
 	}
 
 	output := ioutils.NewWriteFlusher(w)

api/types/registry/authconfig.go

@@ -83,6 +83,8 @@ func DecodeAuthConfig(authEncoded string) (*AuthConfig, error) {
 // Like [DecodeAuthConfig], this function always returns an [AuthConfig], even if an
 // error occurs. It is up to the caller to decide if authentication is required,
 // and if the error can be ignored.
+//
+// Deprecated: this function is no longer used and will be removed in the next release.
 func DecodeAuthConfigBody(rdr io.ReadCloser) (*AuthConfig, error) {
 	return decodeAuthConfigFromReader(rdr)
 }

api/types/registry/authconfig_test.go

@@ -1,8 +1,6 @@
 package registry
 
 import (
-	"io"
-	"strings"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -47,12 +45,6 @@ func TestDecodeAuthConfig(t *testing.T) {
 	})
 }
 
-func TestDecodeAuthConfigBody(t *testing.T) {
-	token, err := DecodeAuthConfigBody(io.NopCloser(strings.NewReader(unencoded)))
-	assert.NilError(t, err)
-	assert.Equal(t, *token, expected)
-}
-
 func TestEncodeAuthConfig(t *testing.T) {
 	token, err := EncodeAuthConfig(expected)
 	assert.NilError(t, err)

client/image_push.go

@@ -66,7 +66,16 @@ func (cli *Client) ImagePush(ctx context.Context, image string, options image.Pu
 }
 
 func (cli *Client) tryImagePush(ctx context.Context, imageID string, query url.Values, registryAuth string) (*http.Response, error) {
-	return cli.post(ctx, "/images/"+imageID+"/push", query, nil, http.Header{
+	// Always send a body (which may be an empty JSON document ("{}")) to prevent
+	// EOF errors on older daemons which had faulty fallback code for handling
+	// authentication in the body when no auth-header was set, resulting in;
+	//
+	//	Error response from daemon: bad parameters and missing X-Registry-Auth: invalid X-Registry-Auth header: EOF
+	//
+	// We use [http.NoBody], which gets marshaled to an empty JSON document.
+	//
+	// see: https://github.com/moby/moby/commit/ea29dffaa541289591aa44fa85d2a596ce860e16
+	return cli.post(ctx, "/images/"+imageID+"/push", query, http.NoBody, http.Header{
 		registry.AuthHeader: {registryAuth},
 	})
 }

hack/buildkit-ref

@@ -19,6 +19,9 @@ if [[ "${buildkit_ref}" == *-*-* ]]; then
 	buildkit_ref=$(curl -s "https://api.github.com/repos/${buildkit_repo}/commits/${buildkit_ref}" | jq -r .sha)
 fi
 
+# FIXME(thaJeztah) temporarily overriding version to use for tests; remove with the next release of buildkit; see https://github.com/moby/moby/issues/50389
+buildkit_ref=dd2b4e18663c58ac3762d7b60b2c3301f71d5fa9
+
 cat << EOF
 BUILDKIT_REPO=$buildkit_repo
 BUILDKIT_REF=$buildkit_ref

integration/networking/bridge_linux_test.go

@@ -185,6 +185,8 @@ func TestBridgeICC(t *testing.T) {
 				Force: true,
 			})
 
+			networking.FirewalldReload(t, d)
+
 			pingHost := tc.pingHost
 			if pingHost == "" {
 				if tc.isLinkLocal {
@@ -319,6 +321,7 @@ func TestBridgeINC(t *testing.T) {
 			defer c.ContainerRemove(ctx, id1, containertypes.RemoveOptions{
 				Force: true,
 			})
+			networking.FirewalldReload(t, d)
 
 			ctr1Info := container.Inspect(ctx, t, c, id1)
 			targetAddr := ctr1Info.NetworkSettings.Networks[bridge1].IPAddress
@@ -457,6 +460,7 @@ func TestBridgeINCRouted(t *testing.T) {
 
 	for _, fwdPolicy := range []string{"ACCEPT", "DROP"} {
 		networking.SetFilterForwardPolicies(t, firewallBackend, fwdPolicy)
+		networking.FirewalldReload(t, d)
 		t.Run(fwdPolicy, func(t *testing.T) {
 			for _, tc := range testcases {
 				t.Run(tc.name+"/v4/ping", func(t *testing.T) {
@@ -574,6 +578,8 @@ func TestRoutedAccessToPublishedPort(t *testing.T) {
 			)
 			defer network.RemoveNoError(ctx, t, c, routedNetName)
 
+			networking.FirewalldReload(t, d)
+
 			// With docker-proxy disabled, a container can't normally access a port published
 			// from a container in a different bridge network. But, users can add rules to
 			// the DOCKER-USER chain to get around that limitation of docker's iptables rules.
@@ -823,6 +829,7 @@ func TestInternalNwConnectivity(t *testing.T) {
 		container.WithNetworkMode(bridgeName),
 	)
 	defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{Force: true})
+	networking.FirewalldReload(t, d)
 
 	execCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
 	defer cancel()
@@ -1000,9 +1007,10 @@ func TestNoIP6Tables(t *testing.T) {
 	ctx := setupTest(t)
 
 	testcases := []struct {
-		name        string
-		option      string
-		expIPTables bool
+		name            string
+		option          string
+		reloadFirewalld bool
+		expIPTables     bool
 	}{
 		{
 			name:        "ip6tables on",
@@ -1013,10 +1021,18 @@ func TestNoIP6Tables(t *testing.T) {
 			name:   "ip6tables off",
 			option: "--ip6tables=false",
 		},
+		{
+			name:            "ip6tables off with firewalld reload",
+			option:          "--ip6tables=false",
+			reloadFirewalld: true,
+		},
 	}
 
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
+			if tc.reloadFirewalld {
+				skip.If(t, !networking.FirewalldRunning(), "firewalld is not running")
+			}
 			ctx := testutil.StartSpan(ctx, t)
 
 			d := daemon.New(t)
@@ -1039,6 +1055,9 @@ func TestNoIP6Tables(t *testing.T) {
 			id := container.Run(ctx, t, c, container.WithNetworkMode(netName))
 			defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{Force: true})
 
+			if tc.reloadFirewalld {
+				networking.FirewalldReload(t, d)
+			}
 			var cmd *exec.Cmd
 			if d.FirewallBackendDriver(t) == "nftables" {
 				cmd = exec.Command("nft", "list", "table", "ip6", "docker-bridges")

libnetwork/drivers/bridge/port_mapping_linux.go

@@ -792,11 +792,20 @@ func releasePortBindings(pbs []portBinding, fwn firewaller.Network) error {
 func (n *bridgeNetwork) reapplyPerPortIptables() {
 	n.Lock()
 	var allPBs []portBinding
+	var allEPs []*bridgeEndpoint
 	for _, ep := range n.endpoints {
 		allPBs = append(allPBs, ep.portMapping...)
+		allEPs = append(allEPs, ep)
 	}
 	n.Unlock()
 
+	for _, ep := range allEPs {
+		netip4, netip6 := ep.netipAddrs()
+		if err := n.firewallerNetwork.AddEndpoint(context.TODO(), netip4, netip6); err != nil {
+			log.G(context.TODO()).Warnf("Failed to reconfigure Endpoint: %s", err)
+		}
+	}
+
 	if err := n.firewallerNetwork.AddPorts(context.Background(), mergeChildHostIPs(allPBs)); err != nil {
 		log.G(context.TODO()).Warnf("Failed to reconfigure NAT: %s", err)
 	}

vendor.mod

@@ -62,7 +62,7 @@ require (
 	github.com/miekg/dns v1.1.66
 	github.com/mistifyio/go-zfs/v3 v3.0.1
 	github.com/mitchellh/copystructure v1.2.0
-	github.com/moby/buildkit v0.23.2
+	github.com/moby/buildkit v0.23.2 // FIXME(thaJeztah): remove override from hack/buildkit-ref when updating.
 	github.com/moby/docker-image-spec v1.3.1
 	github.com/moby/go-archive v0.1.0
 	github.com/moby/ipvs v1.1.0