diff --git a/pilot/pkg/bootstrap/mesh.go b/pilot/pkg/bootstrap/mesh.go index a282d4f64a45..ef3505d578e1 100644 --- a/pilot/pkg/bootstrap/mesh.go +++ b/pilot/pkg/bootstrap/mesh.go @@ -19,6 +19,7 @@ import ( "os" "istio.io/istio/pkg/config/mesh" + "istio.io/istio/pkg/config/mesh/kubemesh" "istio.io/istio/pkg/util/gogoprotomarshal" "istio.io/pkg/filewatcher" "istio.io/pkg/log" @@ -66,7 +67,7 @@ func (s *Server) initMeshConfiguration(args *PilotArgs, fileWatcher filewatcher. // Watch the istio ConfigMap for mesh config changes. // This may be necessary for external Istiod. configMapName := getMeshConfigMapName(args.Revision) - s.environment.Watcher = mesh.NewConfigMapWatcher( + s.environment.Watcher = kubemesh.NewConfigMapWatcher( s.kubeClient, args.Namespace, configMapName, configMapKey) } diff --git a/pilot/pkg/networking/core/v1alpha3/fake.go b/pilot/pkg/networking/core/v1alpha3/fake.go index e8d41bb4affa..eceffaf09d63 100644 --- a/pilot/pkg/networking/core/v1alpha3/fake.go +++ b/pilot/pkg/networking/core/v1alpha3/fake.go @@ -1,3 +1,4 @@ +// +build !agent // Copyright Istio Authors // // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pilot/pkg/serviceregistry/kube/controller/namespacecontroller.go b/pilot/pkg/serviceregistry/kube/controller/namespacecontroller.go index 5e53a7e6a4f6..8ebf73c715a1 100644 --- a/pilot/pkg/serviceregistry/kube/controller/namespacecontroller.go +++ b/pilot/pkg/serviceregistry/kube/controller/namespacecontroller.go @@ -26,7 +26,7 @@ import ( "istio.io/istio/pkg/kube" "istio.io/istio/pkg/queue" - certutil "istio.io/istio/security/pkg/util" + "istio.io/istio/security/pkg/k8s" "istio.io/pkg/log" ) @@ -134,7 +134,7 @@ func (nc *NamespaceController) insertDataForNamespace(ns string) error { Namespace: ns, Labels: configMapLabel, } - return certutil.InsertDataToConfigMap(nc.client, meta, nc.getData()) + return k8s.InsertDataToConfigMap(nc.client, meta, nc.getData()) } // On namespace change, update the config map. @@ -148,7 +148,7 @@ func (nc *NamespaceController) namespaceChange(ns *v1.Namespace) error { // When a config map is changed, merge the data into the configmap func (nc *NamespaceController) configMapChange(cm *v1.ConfigMap) error { - if err := certutil.UpdateDataInConfigMap(nc.client, cm.DeepCopy(), nc.getData()); err != nil { + if err := k8s.UpdateDataInConfigMap(nc.client, cm.DeepCopy(), nc.getData()); err != nil { return fmt.Errorf("error when inserting CA cert to configmap %v: %v", cm.Name, err) } return nil diff --git a/pilot/pkg/serviceregistry/kube/controller/namespacecontroller_test.go b/pilot/pkg/serviceregistry/kube/controller/namespacecontroller_test.go index f42b8e2c126a..916d144be4ec 100644 --- a/pilot/pkg/serviceregistry/kube/controller/namespacecontroller_test.go +++ b/pilot/pkg/serviceregistry/kube/controller/namespacecontroller_test.go @@ -27,7 +27,7 @@ import ( "istio.io/istio/pkg/kube" "istio.io/istio/pkg/test/util/retry" - "istio.io/istio/security/pkg/util" + "istio.io/istio/security/pkg/k8s" ) func TestNamespaceController(t *testing.T) { @@ -45,7 +45,7 @@ func TestNamespaceController(t *testing.T) { expectConfigMap(t, client, "foo", testdata) newData := map[string]string{"key": "value", "foo": "bar"} - if err := util.InsertDataToConfigMap(client.CoreV1(), metav1.ObjectMeta{Name: CACertNamespaceConfigMap, Namespace: "foo"}, newData); err != nil { + if err := k8s.InsertDataToConfigMap(client.CoreV1(), metav1.ObjectMeta{Name: CACertNamespaceConfigMap, Namespace: "foo"}, newData); err != nil { t.Fatal(err) } expectConfigMap(t, client, "foo", newData) diff --git a/pilot/pkg/xds/fake.go b/pilot/pkg/xds/fake.go index 39a256bc69c8..a62c6f8b5776 100644 --- a/pilot/pkg/xds/fake.go +++ b/pilot/pkg/xds/fake.go @@ -1,3 +1,4 @@ +// +build !agent // Copyright Istio Authors // // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/config/mesh/kubemesh/watcher.go b/pkg/config/mesh/kubemesh/watcher.go new file mode 100644 index 000000000000..9aad82e3564e --- /dev/null +++ b/pkg/config/mesh/kubemesh/watcher.go @@ -0,0 +1,70 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kubemesh + +import ( + "fmt" + + v1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/cache" + + meshconfig "istio.io/api/mesh/v1alpha1" + "istio.io/istio/pkg/config/mesh" + "istio.io/istio/pkg/kube" + "istio.io/istio/pkg/kube/configmapwatcher" + "istio.io/pkg/log" +) + +// NewConfigMapWatcher creates a new Watcher for changes to the given ConfigMap. +func NewConfigMapWatcher(client kube.Client, namespace, name, key string) mesh.Watcher { + defaultMesh := mesh.DefaultMeshConfig() + w := &mesh.InternalWatcher{MeshConfig: &defaultMesh} + c := configmapwatcher.NewController(client, namespace, name, func(cm *v1.ConfigMap) { + meshConfig, err := ReadConfigMap(cm, key) + if err != nil { + // Keep the last known config in case there's a misconfiguration issue. + log.Warnf("failed to read mesh config from ConfigMap: %v", err) + return + } + w.HandleMeshConfig(meshConfig) + }) + + stop := make(chan struct{}) + go c.Run(stop) + // Ensure the ConfigMap is initially loaded if present. + cache.WaitForCacheSync(stop, c.HasSynced) + return w +} + +func ReadConfigMap(cm *v1.ConfigMap, key string) (*meshconfig.MeshConfig, error) { + if cm == nil { + log.Info("no ConfigMap found, using default MeshConfig config") + defaultMesh := mesh.DefaultMeshConfig() + return &defaultMesh, nil + } + + cfgYaml, exists := cm.Data[key] + if !exists { + return nil, fmt.Errorf("missing ConfigMap key %q", key) + } + + meshConfig, err := mesh.ApplyMeshConfigDefaults(cfgYaml) + if err != nil { + return nil, fmt.Errorf("failed reading MeshConfig config: %v. YAML:\n%s", err, cfgYaml) + } + + log.Info("Loaded MeshConfig config from Kubernetes API server.") + return meshConfig, nil +} diff --git a/pkg/config/mesh/kubemesh/watcher_test.go b/pkg/config/mesh/kubemesh/watcher_test.go new file mode 100644 index 000000000000..c84e2be375a1 --- /dev/null +++ b/pkg/config/mesh/kubemesh/watcher_test.go @@ -0,0 +1,126 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kubemesh + +import ( + "context" + "fmt" + "sync" + "testing" + "time" + + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + meshconfig "istio.io/api/mesh/v1alpha1" + "istio.io/istio/pkg/config/mesh" + "istio.io/istio/pkg/kube" +) + +const ( + namespace string = "istio-system" + name string = "istio" + key string = "MeshConfig" +) + +func makeConfigMap(resourceVersion string, data map[string]string) *v1.ConfigMap { + return &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + ResourceVersion: resourceVersion, + }, + Data: data, + } +} + +func TestNewConfigMapWatcher(t *testing.T) { + yaml := "trustDomain: something.new" + m, err := mesh.ApplyMeshConfigDefaults(yaml) + if err != nil { + t.Fatal(err) + } + + cm := makeConfigMap("1", map[string]string{ + key: yaml, + }) + badCM := makeConfigMap("2", map[string]string{ + "other-key": yaml, + }) + badCM2 := makeConfigMap("3", map[string]string{ + key: "bad yaml", + }) + + client := kube.NewFakeClient() + cms := client.Kube().CoreV1().ConfigMaps(namespace) + w := NewConfigMapWatcher(client, namespace, name, key) + + defaultMesh := mesh.DefaultMeshConfig() + + var mu sync.Mutex + newM := &defaultMesh + w.AddMeshHandler(func() { + mu.Lock() + defer mu.Unlock() + newM = w.Mesh() + }) + + steps := []struct { + added *v1.ConfigMap + updated *v1.ConfigMap + deleted *v1.ConfigMap + + expect *meshconfig.MeshConfig + }{ + {expect: &defaultMesh}, + {added: cm, expect: m}, + + // Handle misconfiguration errors. + {updated: badCM, expect: m}, + {updated: cm, expect: m}, + {updated: badCM2, expect: m}, + {updated: badCM, expect: m}, + {updated: cm, expect: m}, + + {deleted: cm, expect: &defaultMesh}, + {added: badCM, expect: &defaultMesh}, + } + + for i, step := range steps { + t.Run(fmt.Sprintf("[%v]", i), func(t *testing.T) { + g := NewWithT(t) + + switch { + case step.added != nil: + _, err := cms.Create(context.TODO(), step.added, metav1.CreateOptions{}) + g.Expect(err).Should(BeNil()) + case step.updated != nil: + _, err := cms.Update(context.TODO(), step.updated, metav1.UpdateOptions{}) + g.Expect(err).Should(BeNil()) + case step.deleted != nil: + g.Expect(cms.Delete(context.TODO(), step.deleted.Name, metav1.DeleteOptions{})). + Should(Succeed()) + } + + g.Eventually(w.Mesh).Should(Equal(step.expect)) + g.Eventually(func() *meshconfig.MeshConfig { + mu.Lock() + defer mu.Unlock() + return newM + }, time.Second).Should(Equal(step.expect)) + }) + } +} diff --git a/pkg/config/mesh/watcher.go b/pkg/config/mesh/watcher.go index 4083c212f877..591bb6e32cee 100644 --- a/pkg/config/mesh/watcher.go +++ b/pkg/config/mesh/watcher.go @@ -15,19 +15,14 @@ package mesh import ( - "fmt" "reflect" "sync" "sync/atomic" "unsafe" "github.com/davecgh/go-spew/spew" - v1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/cache" meshconfig "istio.io/api/mesh/v1alpha1" - "istio.io/istio/pkg/kube" - "istio.io/istio/pkg/kube/configmapwatcher" "istio.io/pkg/filewatcher" "istio.io/pkg/log" ) @@ -45,19 +40,19 @@ type Watcher interface { AddMeshHandler(func()) } -var _ Watcher = &watcher{} +var _ Watcher = &InternalWatcher{} -type watcher struct { - mutex sync.Mutex - handlers []func() - mesh *meshconfig.MeshConfig +type InternalWatcher struct { + mutex sync.Mutex + handlers []func() + MeshConfig *meshconfig.MeshConfig } // NewFixedWatcher creates a new Watcher that always returns the given mesh config. It will never // fire any events, since the config never changes. func NewFixedWatcher(mesh *meshconfig.MeshConfig) Watcher { - return &watcher{ - mesh: mesh, + return &InternalWatcher{ + MeshConfig: mesh, } } @@ -69,8 +64,8 @@ func NewFileWatcher(fileWatcher filewatcher.FileWatcher, filename string) (Watch return nil, err } - w := &watcher{ - mesh: meshConfig, + w := &InternalWatcher{ + MeshConfig: meshConfig, } // Watch the config file for changes and reload if it got modified @@ -81,56 +76,35 @@ func NewFileWatcher(fileWatcher filewatcher.FileWatcher, filename string) (Watch log.Warnf("failed to read mesh configuration, using default: %v", err) return } - w.handleMeshConfig(meshConfig) + w.HandleMeshConfig(meshConfig) }) return w, nil } -// NewConfigMapWatcher creates a new Watcher for changes to the given ConfigMap. -func NewConfigMapWatcher(client kube.Client, namespace, name, key string) Watcher { - defaultMesh := DefaultMeshConfig() - w := &watcher{mesh: &defaultMesh} - c := configmapwatcher.NewController(client, namespace, name, func(cm *v1.ConfigMap) { - meshConfig, err := ReadConfigMap(cm, key) - if err != nil { - // Keep the last known config in case there's a misconfiguration issue. - log.Warnf("failed to read mesh config from ConfigMap: %v", err) - return - } - w.handleMeshConfig(meshConfig) - }) - - stop := make(chan struct{}) - go c.Run(stop) - // Ensure the ConfigMap is initially loaded if present. - cache.WaitForCacheSync(stop, c.HasSynced) - return w -} - // Mesh returns the latest mesh config. -func (w *watcher) Mesh() *meshconfig.MeshConfig { - return (*meshconfig.MeshConfig)(atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&w.mesh)))) +func (w *InternalWatcher) Mesh() *meshconfig.MeshConfig { + return (*meshconfig.MeshConfig)(atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&w.MeshConfig)))) } // AddMeshHandler registers a callback handler for changes to the mesh config. -func (w *watcher) AddMeshHandler(h func()) { +func (w *InternalWatcher) AddMeshHandler(h func()) { w.mutex.Lock() defer w.mutex.Unlock() w.handlers = append(w.handlers, h) } -func (w *watcher) handleMeshConfig(meshConfig *meshconfig.MeshConfig) { +func (w *InternalWatcher) HandleMeshConfig(meshConfig *meshconfig.MeshConfig) { var handlers []func() w.mutex.Lock() - if !reflect.DeepEqual(meshConfig, w.mesh) { + if !reflect.DeepEqual(meshConfig, w.MeshConfig) { log.Infof("mesh configuration updated to: %s", spew.Sdump(meshConfig)) - if !reflect.DeepEqual(meshConfig.ConfigSources, w.mesh.ConfigSources) { + if !reflect.DeepEqual(meshConfig.ConfigSources, w.MeshConfig.ConfigSources) { log.Info("mesh configuration sources have changed") // TODO Need to recreate or reload initConfigController() } - atomic.StorePointer((*unsafe.Pointer)(unsafe.Pointer(&w.mesh)), unsafe.Pointer(meshConfig)) + atomic.StorePointer((*unsafe.Pointer)(unsafe.Pointer(&w.MeshConfig)), unsafe.Pointer(meshConfig)) handlers = append(handlers, w.handlers...) } w.mutex.Unlock() @@ -139,24 +113,3 @@ func (w *watcher) handleMeshConfig(meshConfig *meshconfig.MeshConfig) { h() } } - -func ReadConfigMap(cm *v1.ConfigMap, key string) (*meshconfig.MeshConfig, error) { - if cm == nil { - log.Info("no ConfigMap found, using default mesh config") - defaultMesh := DefaultMeshConfig() - return &defaultMesh, nil - } - - cfgYaml, exists := cm.Data[key] - if !exists { - return nil, fmt.Errorf("missing ConfigMap key %q", key) - } - - meshConfig, err := ApplyMeshConfigDefaults(cfgYaml) - if err != nil { - return nil, fmt.Errorf("failed reading mesh config: %v. YAML:\n%s", err, cfgYaml) - } - - log.Info("Loaded mesh config from Kubernetes API server.") - return meshConfig, nil -} diff --git a/pkg/config/mesh/watcher_test.go b/pkg/config/mesh/watcher_test.go index b798b4b0a50b..71b53fc04178 100644 --- a/pkg/config/mesh/watcher_test.go +++ b/pkg/config/mesh/watcher_test.go @@ -15,24 +15,18 @@ package mesh_test import ( - "context" - "fmt" "io" "io/ioutil" "os" "path/filepath" - "sync" "testing" "time" "github.com/golang/protobuf/proto" . "github.com/onsi/gomega" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" meshconfig "istio.io/api/mesh/v1alpha1" "istio.io/istio/pkg/config/mesh" - "istio.io/istio/pkg/kube" "istio.io/istio/pkg/util/protomarshal" "istio.io/pkg/filewatcher" ) @@ -146,98 +140,3 @@ func BenchmarkGetMesh(b *testing.B) { handler(w.Mesh()) } } - -const ( - namespace string = "istio-system" - name string = "istio" - key string = "mesh" -) - -func makeConfigMap(resourceVersion string, data map[string]string) *v1.ConfigMap { - return &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - ResourceVersion: resourceVersion, - }, - Data: data, - } -} - -func TestNewConfigMapWatcher(t *testing.T) { - yaml := "trustDomain: something.new" - m, err := mesh.ApplyMeshConfigDefaults(yaml) - if err != nil { - t.Fatal(err) - } - - cm := makeConfigMap("1", map[string]string{ - key: yaml, - }) - badCM := makeConfigMap("2", map[string]string{ - "other-key": yaml, - }) - badCM2 := makeConfigMap("3", map[string]string{ - key: "bad yaml", - }) - - client := kube.NewFakeClient() - cms := client.Kube().CoreV1().ConfigMaps(namespace) - w := mesh.NewConfigMapWatcher(client, namespace, name, key) - - defaultMesh := mesh.DefaultMeshConfig() - - var mu sync.Mutex - newM := &defaultMesh - w.AddMeshHandler(func() { - mu.Lock() - defer mu.Unlock() - newM = w.Mesh() - }) - - steps := []struct { - added *v1.ConfigMap - updated *v1.ConfigMap - deleted *v1.ConfigMap - - expect *meshconfig.MeshConfig - }{ - {expect: &defaultMesh}, - {added: cm, expect: m}, - - // Handle misconfiguration errors. - {updated: badCM, expect: m}, - {updated: cm, expect: m}, - {updated: badCM2, expect: m}, - {updated: badCM, expect: m}, - {updated: cm, expect: m}, - - {deleted: cm, expect: &defaultMesh}, - {added: badCM, expect: &defaultMesh}, - } - - for i, step := range steps { - t.Run(fmt.Sprintf("[%v]", i), func(t *testing.T) { - g := NewWithT(t) - - switch { - case step.added != nil: - _, err := cms.Create(context.TODO(), step.added, metav1.CreateOptions{}) - g.Expect(err).Should(BeNil()) - case step.updated != nil: - _, err := cms.Update(context.TODO(), step.updated, metav1.UpdateOptions{}) - g.Expect(err).Should(BeNil()) - case step.deleted != nil: - g.Expect(cms.Delete(context.TODO(), step.deleted.Name, metav1.DeleteOptions{})). - Should(Succeed()) - } - - g.Eventually(w.Mesh).Should(Equal(step.expect)) - g.Eventually(func() *meshconfig.MeshConfig { - mu.Lock() - defer mu.Unlock() - return newM - }, time.Second).Should(Equal(step.expect)) - }) - } -} diff --git a/security/pkg/util/configutil.go b/security/pkg/k8s/configutil.go similarity index 99% rename from security/pkg/util/configutil.go rename to security/pkg/k8s/configutil.go index 99f93cc67f40..63fdded78aa5 100644 --- a/security/pkg/util/configutil.go +++ b/security/pkg/k8s/configutil.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package util +package k8s import ( "context" diff --git a/security/pkg/util/configutil_test.go b/security/pkg/k8s/configutil_test.go similarity index 99% rename from security/pkg/util/configutil_test.go rename to security/pkg/k8s/configutil_test.go index 61f74261d701..1286b2744e43 100644 --- a/security/pkg/util/configutil_test.go +++ b/security/pkg/k8s/configutil_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package util +package k8s import ( "context" diff --git a/tests/binary/binaries_test.go b/tests/binary/binaries_test.go index 94e04360d3cc..dbbab10e5933 100644 --- a/tests/binary/binaries_test.go +++ b/tests/binary/binaries_test.go @@ -92,7 +92,7 @@ func TestBinarySizes(t *testing.T) { // TODO: shrink the ranges here once the active work to reduce binary size is complete // For now, having two small a range will result in lots of "merge conflicts" "istioctl": {60, 100}, - "pilot-agent": {30, 80}, + "pilot-agent": {30, 45}, "pilot-discovery": {60, 80}, "bug-report": {60, 100}, } @@ -108,6 +108,7 @@ func TestBinarySizes(t *testing.T) { t.Fatal(err) } got := fi.Size() / (1000 * 1000) + t.Logf("Actual size: %dmb. Range: [%dmb, %dmb]", got, tt.minMb, tt.maxMb) if got > tt.maxMb { t.Fatalf("Binary size of %dmb was greater than max allowed size %dmb", got, tt.maxMb) }