Skip to content

Commit

Permalink
types: allow docker bridge network range except on libvirt
Browse files Browse the repository at this point in the history
The networking team has made an argument that not allowing docker default network bridge for machine, service and cluster networks is
an invalid validation see https://bugzilla.redhat.com/show_bug.cgi?id=1858342#c2 . So we are dropping that validation.

But on libvirt platform overlap with docker bridge has known to cause problems therefore we are keeping the validation for this platform.

Also when this network range is in use, warning is shown to the user so that in case users keep weird errors we can quickly traige to networking team.
  • Loading branch information
abhinavdahiya committed Jul 30, 2020
1 parent 30e821f commit 5dac65e
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 4 deletions.
43 changes: 43 additions & 0 deletions pkg/types/validation/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

dockerref "github.com/containers/image/docker/reference"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"

Expand Down Expand Up @@ -79,6 +80,7 @@ func ValidateInstallConfig(c *types.InstallConfig) field.ErrorList {
if c.Networking != nil {
allErrs = append(allErrs, validateNetworking(c.Networking, field.NewPath("networking"))...)
allErrs = append(allErrs, validateNetworkingIPVersion(c.Networking, &c.Platform)...)
allErrs = append(allErrs, validateNetworkingForPlatform(c.Networking, &c.Platform, field.NewPath("networking"))...)
} else {
allErrs = append(allErrs, field.Required(field.NewPath("networking"), "networking is required"))
}
Expand Down Expand Up @@ -279,6 +281,47 @@ func validateNetworking(n *types.Networking, fldPath *field.Path) field.ErrorLis
return allErrs
}

func validateNetworkingForPlatform(n *types.Networking, platform *types.Platform, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
switch {
case platform.Libvirt != nil:
errMsg := "overlaps with default Docker Bridge subnet"
for idx, mn := range n.MachineNetwork {
if validate.DoCIDRsOverlap(&mn.CIDR.IPNet, validate.DockerBridgeCIDR) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("machineNewtork").Index(idx), mn.CIDR.String(), errMsg))
}
}
for idx, sn := range n.ServiceNetwork {
if validate.DoCIDRsOverlap(&sn.IPNet, validate.DockerBridgeCIDR) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceNetwork").Index(idx), sn.String(), errMsg))
}
}
for idx, cn := range n.ClusterNetwork {
if validate.DoCIDRsOverlap(&cn.CIDR.IPNet, validate.DockerBridgeCIDR) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("clusterNetwork").Index(idx), cn.CIDR.String(), errMsg))
}
}
default:
warningMsgFmt := "%s: %s overlaps with default Docker Bridge subnet"
for idx, mn := range n.MachineNetwork {
if validate.DoCIDRsOverlap(&mn.CIDR.IPNet, validate.DockerBridgeCIDR) {
logrus.Warnf(warningMsgFmt, fldPath.Child("machineNetwork").Index(idx), mn.CIDR.String())
}
}
for idx, sn := range n.ServiceNetwork {
if validate.DoCIDRsOverlap(&sn.IPNet, validate.DockerBridgeCIDR) {
logrus.Warnf(warningMsgFmt, fldPath.Child("serviceNetwork").Index(idx), sn.String())
}
}
for idx, cn := range n.ClusterNetwork {
if validate.DoCIDRsOverlap(&cn.CIDR.IPNet, validate.DockerBridgeCIDR) {
logrus.Warnf(warningMsgFmt, fldPath.Child("clusterNetwork").Index(idx), cn.CIDR.String())
}
}
}
return allErrs
}

func validateClusterNetwork(n *types.Networking, cn *types.ClusterNetworkEntry, idx int, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if err := validate.SubnetCIDR(&cn.CIDR.IPNet); err != nil {
Expand Down
19 changes: 19 additions & 0 deletions pkg/types/validation/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,25 @@ func TestValidateInstallConfig(t *testing.T) {
}(),
expectedError: `^credentialsMode: Unsupported value: "bad-mode": supported values: "manual", "mint", "passthrough"$`,
},
{
name: "allowed docker bridge with non-libvirt",
installConfig: func() *types.InstallConfig {
c := validInstallConfig()
c.Networking.MachineNetwork = []types.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.17.64.0/18")}}
return c
}(),
expectedError: ``,
},
{
name: "docker bridge not allowed with libvirt",
installConfig: func() *types.InstallConfig {
c := validInstallConfig()
c.Platform = types.Platform{Libvirt: validLibvirtPlatform()}
c.Networking.MachineNetwork = []types.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.17.64.0/18")}}
return c
}(),
expectedError: `\Q[networking.machineNewtork[0]: Invalid value: "172.17.64.0/18": overlaps with default Docker Bridge subnet, platform: Invalid value: "libvirt": must specify one of the platforms (\E.*\Q)]\E`,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
6 changes: 2 additions & 4 deletions pkg/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import (
)

var (
dockerBridgeCIDR = func() *net.IPNet {
// DockerBridgeCIDR is the network range that is used by default network for docker.
DockerBridgeCIDR = func() *net.IPNet {
_, cidr, _ := net.ParseCIDR("172.17.0.0/16")
return cidr
}()
Expand Down Expand Up @@ -134,9 +135,6 @@ func SubnetCIDR(cidr *net.IPNet) error {
if nip.String() != cidr.IP.String() {
return fmt.Errorf("invalid network address. got %s, expecting %s", cidr.String(), (&net.IPNet{IP: nip, Mask: cidr.Mask}).String())
}
if DoCIDRsOverlap(cidr, dockerBridgeCIDR) {
return fmt.Errorf("overlaps with default Docker Bridge subnet (%v)", cidr.String())
}
return nil
}

Expand Down

0 comments on commit 5dac65e

Please sign in to comment.