Skip to content

Commit

Permalink
fix: correctly parse multiple pod/service CIDRs
Browse files Browse the repository at this point in the history
This changes machinery API for the configuration to make it more
obvious that the returned value is a list of CIDRs and adjusts usage
accordingly.

For the K8s Address Filter controller, fix the actual bug by parsing
CIDRs as a list of values.

Fixes #4192

Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>
  • Loading branch information
smira committed Sep 8, 2021
1 parent 69897db commit 14c69df
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 43 deletions.
30 changes: 21 additions & 9 deletions internal/app/machined/pkg/controllers/config/k8s_address_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,22 +71,34 @@ func (ctrl *K8sAddressFilterController) Run(ctx context.Context, r controller.Ru
if cfg != nil {
cfgProvider := cfg.(*config.MachineConfig).Config()

var podCIDR, serviceCIDR netaddr.IPPrefix
var podCIDRs, serviceCIDRs []netaddr.IPPrefix

podCIDR, err = netaddr.ParseIPPrefix(cfgProvider.Cluster().Network().PodCIDR())
if err != nil {
return fmt.Errorf("error parsing podCIDR: %w", err)
for _, cidr := range cfgProvider.Cluster().Network().PodCIDRs() {
var ipPrefix netaddr.IPPrefix

ipPrefix, err = netaddr.ParseIPPrefix(cidr)
if err != nil {
return fmt.Errorf("error parsing podCIDR: %w", err)
}

podCIDRs = append(podCIDRs, ipPrefix)
}

serviceCIDR, err = netaddr.ParseIPPrefix(cfgProvider.Cluster().Network().ServiceCIDR())
if err != nil {
return fmt.Errorf("error parsing serviceCIDR: %w", err)
for _, cidr := range cfgProvider.Cluster().Network().ServiceCIDRs() {
var ipPrefix netaddr.IPPrefix

ipPrefix, err = netaddr.ParseIPPrefix(cidr)
if err != nil {
return fmt.Errorf("error parsing serviceCIDR: %w", err)
}

serviceCIDRs = append(serviceCIDRs, ipPrefix)
}

if err = r.Modify(ctx, network.NewNodeAddressFilter(network.NamespaceName, k8s.NodeAddressFilterNoK8s), func(r resource.Resource) error {
spec := r.(*network.NodeAddressFilter).TypedSpec()

spec.ExcludeSubnets = []netaddr.IPPrefix{podCIDR, serviceCIDR}
spec.ExcludeSubnets = append(append([]netaddr.IPPrefix(nil), podCIDRs...), serviceCIDRs...)

return nil
}); err != nil {
Expand All @@ -98,7 +110,7 @@ func (ctrl *K8sAddressFilterController) Run(ctx context.Context, r controller.Ru
if err = r.Modify(ctx, network.NewNodeAddressFilter(network.NamespaceName, k8s.NodeAddressFilterOnlyK8s), func(r resource.Resource) error {
spec := r.(*network.NodeAddressFilter).TypedSpec()

spec.IncludeSubnets = []netaddr.IPPrefix{podCIDR, serviceCIDR}
spec.IncludeSubnets = append(append([]netaddr.IPPrefix(nil), podCIDRs...), serviceCIDRs...)

return nil
}); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ func (suite *K8sAddressFilterSuite) TestReconcile() {
},
ClusterNetwork: &v1alpha1.ClusterNetworkConfig{
ServiceSubnet: []string{
"10.96.0.0/12",
"10.200.0.0/22",
"fd40:10:200::/112",
},
PodSubnet: []string{
"10.244.0.0/12",
"10.32.0.0/12",
"fd00:10:32::/102",
},
},
},
Expand All @@ -113,7 +115,7 @@ func (suite *K8sAddressFilterSuite) TestReconcile() {
func(res resource.Resource) error {
spec := res.(*network.NodeAddressFilter).TypedSpec()

suite.Assert().Equal("[10.244.0.0/12 10.96.0.0/12]", fmt.Sprintf("%s", spec.IncludeSubnets))
suite.Assert().Equal("[10.32.0.0/12 fd00:10:32::/102 10.200.0.0/22 fd40:10:200::/112]", fmt.Sprintf("%s", spec.IncludeSubnets))
suite.Assert().Empty(spec.ExcludeSubnets)

return nil
Expand All @@ -128,7 +130,7 @@ func (suite *K8sAddressFilterSuite) TestReconcile() {
spec := res.(*network.NodeAddressFilter).TypedSpec()

suite.Assert().Empty(spec.IncludeSubnets)
suite.Assert().Equal("[10.244.0.0/12 10.96.0.0/12]", fmt.Sprintf("%s", spec.ExcludeSubnets))
suite.Assert().Equal("[10.32.0.0/12 fd00:10:32::/102 10.200.0.0/22 fd40:10:200::/112]", fmt.Sprintf("%s", spec.ExcludeSubnets))

return nil
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package config
import (
"context"
"fmt"
"strings"

"github.com/AlekSi/pointer"
"github.com/cosi-project/runtime/pkg/controller"
Expand Down Expand Up @@ -146,7 +145,7 @@ func (ctrl *K8sControlPlaneController) manageAPIServerConfig(ctx context.Context
ControlPlaneEndpoint: cfgProvider.Cluster().Endpoint().String(),
EtcdServers: []string{"https://127.0.0.1:2379"},
LocalPort: cfgProvider.Cluster().LocalAPIServerPort(),
ServiceCIDR: cfgProvider.Cluster().Network().ServiceCIDR(),
ServiceCIDRs: cfgProvider.Cluster().Network().ServiceCIDRs(),
ExtraArgs: cfgProvider.Cluster().APIServer().ExtraArgs(),
ExtraVolumes: convertVolumes(cfgProvider.Cluster().APIServer().ExtraVolumes()),
PodSecurityPolicyEnabled: !cfgProvider.Cluster().APIServer().DisablePodSecurityPolicy(),
Expand All @@ -166,8 +165,8 @@ func (ctrl *K8sControlPlaneController) manageControllerManagerConfig(ctx context
r.(*config.K8sControlPlane).SetControllerManager(config.K8sControlPlaneControllerManagerSpec{
Image: cfgProvider.Cluster().ControllerManager().Image(),
CloudProvider: cloudProvider,
PodCIDR: cfgProvider.Cluster().Network().PodCIDR(),
ServiceCIDR: cfgProvider.Cluster().Network().ServiceCIDR(),
PodCIDRs: cfgProvider.Cluster().Network().PodCIDRs(),
ServiceCIDRs: cfgProvider.Cluster().Network().ServiceCIDRs(),
ExtraArgs: cfgProvider.Cluster().ControllerManager().ExtraArgs(),
ExtraVolumes: convertVolumes(cfgProvider.Cluster().ControllerManager().ExtraVolumes()),
})
Expand Down Expand Up @@ -218,8 +217,7 @@ func (ctrl *K8sControlPlaneController) manageManifestsConfig(ctx context.Context
Server: cfgProvider.Cluster().Endpoint().String(),
ClusterDomain: cfgProvider.Cluster().Network().DNSDomain(),

PodCIDRs: cfgProvider.Cluster().Network().PodCIDR(),
FirstPodCIDR: strings.Split(cfgProvider.Cluster().Network().PodCIDR(), ",")[0],
PodCIDRs: cfgProvider.Cluster().Network().PodCIDRs(),

ProxyEnabled: cfgProvider.Cluster().Proxy().Enabled(),
ProxyImage: cfgProvider.Cluster().Proxy().Image(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (ctrl *ControlPlaneStaticPodController) manageAPIServer(ctx context.Context
fmt.Sprintf("--service-account-issuer=%s", cfg.ControlPlaneEndpoint),
fmt.Sprintf("--service-account-key-file=%s", filepath.Join(constants.KubernetesAPIServerSecretsDir, "service-account.pub")),
fmt.Sprintf("--service-account-signing-key-file=%s", filepath.Join(constants.KubernetesAPIServerSecretsDir, "service-account.key")),
fmt.Sprintf("--service-cluster-ip-range=%s", cfg.ServiceCIDR),
fmt.Sprintf("--service-cluster-ip-range=%s", strings.Join(cfg.ServiceCIDRs, ",")),
fmt.Sprintf("--tls-cert-file=%s", filepath.Join(constants.KubernetesAPIServerSecretsDir, "apiserver.crt")),
fmt.Sprintf("--tls-private-key-file=%s", filepath.Join(constants.KubernetesAPIServerSecretsDir, "apiserver.key")),
"--kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname",
Expand Down Expand Up @@ -327,8 +327,8 @@ func (ctrl *ControlPlaneStaticPodController) manageControllerManager(ctx context
"--allocate-node-cidrs=true",
"--bind-address=127.0.0.1",
"--port=0",
fmt.Sprintf("--cluster-cidr=%s", cfg.PodCIDR),
fmt.Sprintf("--service-cluster-ip-range=%s", cfg.ServiceCIDR),
fmt.Sprintf("--cluster-cidr=%s", strings.Join(cfg.PodCIDRs, ",")),
fmt.Sprintf("--service-cluster-ip-range=%s", strings.Join(cfg.ServiceCIDRs, ",")),
fmt.Sprintf("--cluster-signing-cert-file=%s", filepath.Join(constants.KubernetesControllerManagerSecretsDir, "ca.crt")),
fmt.Sprintf("--cluster-signing-key-file=%s", filepath.Join(constants.KubernetesControllerManagerSecretsDir, "ca.key")),
"--controllers=*,tokencleaner",
Expand Down
2 changes: 2 additions & 0 deletions internal/app/machined/pkg/controllers/k8s/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"context"
"encoding/json"
"fmt"
"strings"
"text/template"

"github.com/AlekSi/pointer"
Expand Down Expand Up @@ -225,6 +226,7 @@ func (ctrl *ManifestController) render(cfg config.K8sManifestsSpec, scrt *secret
tmpl, err := template.New(defaultManifests[i].name).
Funcs(template.FuncMap{
"json": jsonify,
"join": strings.Join,
}).
Parse(string(defaultManifests[i].template))
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions internal/app/machined/pkg/controllers/k8s/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ var defaultManifestSpec = config.K8sManifestsSpec{
Server: "127.0.0.1",
ClusterDomain: "cluster.",

PodCIDRs: constants.DefaultIPv4PodNet,
FirstPodCIDR: constants.DefaultIPv4PodNet,
PodCIDRs: []string{constants.DefaultIPv4PodNet},

ProxyEnabled: true,
ProxyImage: "foo/bar",
Expand Down
4 changes: 2 additions & 2 deletions internal/app/machined/pkg/controllers/k8s/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ spec:
image: {{ .ProxyImage }}
command:
- /usr/local/bin/kube-proxy
- --cluster-cidr={{ .PodCIDRs }}
- --cluster-cidr={{ join .PodCIDRs "," }}
- --hostname-override=$(NODE_NAME)
- --kubeconfig=/etc/kubernetes/kubeconfig
- --proxy-mode={{ .ProxyMode }}
Expand Down Expand Up @@ -546,7 +546,7 @@ data:
}
net-conf.json: |
{
"Network": "{{ .FirstPodCIDR }}",
"Network": "{{ index .PodCIDRs 0 }}",
"Backend": {
"Type": "vxlan",
"Port": 4789
Expand Down
4 changes: 2 additions & 2 deletions pkg/machinery/config/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ type ClusterConfig interface {
// network options.
type ClusterNetwork interface {
CNI() CNI
PodCIDR() string
ServiceCIDR() string
PodCIDRs() []string
ServiceCIDRs() []string
DNSDomain() string
// APIServerIPs returns kube-apiserver IPs in the ServiceCIDR.
APIServerIPs() ([]net.IP, error)
Expand Down
20 changes: 10 additions & 10 deletions pkg/machinery/config/types/v1alpha1/v1alpha1_clusterconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,28 +202,28 @@ func (c *ClusterConfig) CNI() config.CNI {
return c.ClusterNetwork.CNI
}

// PodCIDR implements the config.ClusterNetwork interface.
func (c *ClusterConfig) PodCIDR() string {
// PodCIDRs implements the config.ClusterNetwork interface.
func (c *ClusterConfig) PodCIDRs() []string {
switch {
case c.ClusterNetwork == nil:
fallthrough
case len(c.ClusterNetwork.PodSubnet) == 0:
return constants.DefaultIPv4PodNet
return []string{constants.DefaultIPv4PodNet}
}

return strings.Join(c.ClusterNetwork.PodSubnet, ",")
return c.ClusterNetwork.PodSubnet
}

// ServiceCIDR implements the config.ClusterNetwork interface.
func (c *ClusterConfig) ServiceCIDR() string {
// ServiceCIDRs implements the config.ClusterNetwork interface.
func (c *ClusterConfig) ServiceCIDRs() []string {
switch {
case c.ClusterNetwork == nil:
fallthrough
case len(c.ClusterNetwork.ServiceSubnet) == 0:
return constants.DefaultIPv4ServiceNet
return []string{constants.DefaultIPv4ServiceNet}
}

return strings.Join(c.ClusterNetwork.ServiceSubnet, ",")
return c.ClusterNetwork.ServiceSubnet
}

// DNSDomain implements the config.ClusterNetwork interface.
Expand All @@ -237,7 +237,7 @@ func (c *ClusterConfig) DNSDomain() string {

// APIServerIPs implements the config.ClusterNetwork interface.
func (c *ClusterConfig) APIServerIPs() ([]net.IP, error) {
serviceCIDRs, err := talosnet.SplitCIDRs(c.ServiceCIDR())
serviceCIDRs, err := talosnet.SplitCIDRs(strings.Join(c.ServiceCIDRs(), ","))
if err != nil {
return nil, fmt.Errorf("failed to process Service CIDRs: %w", err)
}
Expand All @@ -247,7 +247,7 @@ func (c *ClusterConfig) APIServerIPs() ([]net.IP, error) {

// DNSServiceIPs implements the config.ClusterNetwork interface.
func (c *ClusterConfig) DNSServiceIPs() ([]net.IP, error) {
serviceCIDRs, err := talosnet.SplitCIDRs(c.ServiceCIDR())
serviceCIDRs, err := talosnet.SplitCIDRs(strings.Join(c.ServiceCIDRs(), ","))
if err != nil {
return nil, fmt.Errorf("failed to process Service CIDRs: %w", err)
}
Expand Down
9 changes: 4 additions & 5 deletions pkg/resources/config/k8s_control_plane.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type K8sControlPlaneAPIServerSpec struct {
ControlPlaneEndpoint string `yaml:"controlPlaneEndpoint"`
EtcdServers []string `yaml:"etcdServers"`
LocalPort int `yaml:"localPort"`
ServiceCIDR string `yaml:"serviceCIDR"`
ServiceCIDRs []string `yaml:"serviceCIDR"`
ExtraArgs map[string]string `yaml:"extraArgs"`
ExtraVolumes []K8sExtraVolume `yaml:"extraVolumes"`
PodSecurityPolicyEnabled bool `yaml:"podSecurityPolicyEnabled"`
Expand All @@ -61,8 +61,8 @@ type K8sControlPlaneAPIServerSpec struct {
type K8sControlPlaneControllerManagerSpec struct {
Image string `yaml:"image"`
CloudProvider string `yaml:"cloudProvider"`
PodCIDR string `yaml:"podCIDR"`
ServiceCIDR string `yaml:"serviceCIDR"`
PodCIDRs []string `yaml:"podCIDRs"`
ServiceCIDRs []string `yaml:"serviceCIDRs"`
ExtraArgs map[string]string `yaml:"extraArgs"`
ExtraVolumes []K8sExtraVolume `yaml:"extraVolumes"`
}
Expand All @@ -81,8 +81,7 @@ type K8sManifestsSpec struct {
Server string `yaml:"string"`
ClusterDomain string `yaml:"clusterDomain"`

PodCIDRs string `yaml:"podCIDRs"`
FirstPodCIDR string `yaml:"firstPodCIDR"`
PodCIDRs []string `yaml:"podCIDRs"`

ProxyEnabled bool `yaml:"proxyEnabled"`
ProxyImage string `yaml:"proxyImage"`
Expand Down

0 comments on commit 14c69df

Please sign in to comment.