Merge remote-tracking branch 'origin' into alias

changelog/v0.17.2/fix-equality-segfault.yaml

@@ -0,0 +1,6 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/skv2/issues/207
+    description: >-
+      Fix equality for protos, since we were using the `proto.Equals` on k8s resources that implemented the interface
+      but should have been using reflection.

changelog/v0.17.2/kubeconfig-loader-check-recommended.yaml

@@ -0,0 +1,4 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/skv2/issues/205
+    description: Add recommended kubeconfig directory path to kubeconfig loader loading rules.

pkg/controllerutils/equality_test.go

@@ -5,6 +5,7 @@ import (
 	. "github.com/onsi/gomega"
 	things_test_io_v1 "github.com/solo-io/skv2/codegen/test/api/things.test.io/v1"
 	v1 "k8s.io/api/core/v1"
+	rbac_authorization_k8s_io_v1 "k8s.io/api/rbac/v1"
 	k8s_meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	. "github.com/solo-io/skv2/pkg/controllerutils"
@@ -81,8 +82,7 @@ var _ = Describe("ObjectsEqual", func() {
 		equal := ObjectsEqual(obj1, obj2)
 		Expect(equal).To(BeFalse())
 	})
-
-	It("asserts equality on two proto.Message objects even if they are passed in by value", func() {
+	It("asserts equality on two proto.Message objects (protov2, with unknown fields) even if they are passed in by reference", func() {
 		obj1 := &things_test_io_v1.Paint{
 			Spec: things_test_io_v1.PaintSpec{
 				Color: &things_test_io_v1.PaintColor{
@@ -107,6 +107,46 @@ var _ = Describe("ObjectsEqual", func() {
 		equal := ObjectsEqual(obj1, obj2)
 		Expect(equal).To(BeTrue())
 	})
+	It("asserts equality on two proto.Message objects even if they are 'fake' (i.e., k8s protos implementing github protov1, not google protov2 protos)", func() {
+		obj1 := &rbac_authorization_k8s_io_v1.RoleBinding{
+			TypeMeta: k8s_meta.TypeMeta{},
+			ObjectMeta: k8s_meta.ObjectMeta{
+				Name:      "sample-role-binding",
+				Namespace: "default",
+				Labels:    map[string]string{"k": "v"},
+			},
+			Subjects: []rbac_authorization_k8s_io_v1.Subject{{
+				Kind:      "ServiceAccount",
+				Name:      "sample-target",
+				Namespace: "default",
+			}},
+			RoleRef: rbac_authorization_k8s_io_v1.RoleRef{
+				APIGroup: "rbac.authorization.k8s.io",
+				Kind:     "Role",
+				Name:     "sample-controller",
+			},
+		}
+		obj2 := &rbac_authorization_k8s_io_v1.RoleBinding{
+			TypeMeta: k8s_meta.TypeMeta{},
+			ObjectMeta: k8s_meta.ObjectMeta{
+				Name:      "sample-role-binding",
+				Namespace: "default",
+				Labels:    map[string]string{"k": "v"},
+			},
+			Subjects: []rbac_authorization_k8s_io_v1.Subject{{
+				Kind:      "ServiceAccount",
+				Name:      "sample-target",
+				Namespace: "default",
+			}},
+			RoleRef: rbac_authorization_k8s_io_v1.RoleRef{
+				APIGroup: "rbac.authorization.k8s.io",
+				Kind:     "Role",
+				Name:     "sample-controller",
+			},
+		}
+		equal := ObjectsEqual(obj1, obj2)
+		Expect(equal).To(BeTrue())
+	})
 })
 
 var _ = Describe("ObjectStatusesEqual", func() {

pkg/equalityutils/deep_equal.go

@@ -3,19 +3,19 @@ package equalityutils
 import (
 	"reflect"
 
-	"github.com/golang/protobuf/proto"
+	protoV2 "google.golang.org/protobuf/proto"
 )
 
 // DeepEqual should be used in place of reflect.DeepEqual when the type of an object is unknown and may be a proto message.
 // see https://github.com/golang/protobuf/issues/1173 for details on why reflect.DeepEqual no longer works for proto messages
 func DeepEqual(val1, val2 interface{}) bool {
-	protoVal1, isProto := val1.(proto.Message)
+	protoVal1, isProto := val1.(protoV2.Message)
 	if isProto {
-		protoVal2, isProto := val2.(proto.Message)
+		protoVal2, isProto := val2.(protoV2.Message)
 		if !isProto {
 			return false // different types
 		}
-		return proto.Equal(protoVal1, protoVal2)
+		return protoV2.Equal(protoVal1, protoVal2)
 	}
 	return reflect.DeepEqual(val1, val2)
 }

pkg/multicluster/kubeconfig/loader.go

@@ -1,33 +1,23 @@
 package kubeconfig
 
 import (
-	"fmt"
-	"os"
-	"os/user"
-	"path"
-
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 )
 
 // Fetch ClientConfig. If kubeConfigPath is not specified, retrieve the kubeconfig from environment in which this is invoked.
 // Override the API Server URL and current context if specified.
-// Copied and modified from https://github.com/kubernetes-sigs/controller-runtime/blob/cb7f85860a8cde7259b35bb84af1fdcb02c098f2/pkg/client/config/config.go#L135
 func GetClientConfigWithContext(kubeConfigPath, kubeContext, apiServerUrl string) (clientcmd.ClientConfig, error) {
+
+	// default loading rules checks for KUBECONFIG env var
 	loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
+	// also check recommended default kubeconfig file locations
+	loadingRules.Precedence = append(loadingRules.Precedence, clientcmd.RecommendedHomeFile)
 
+	// explicit path overrides all loading rules, will error if not found
 	if kubeConfigPath != "" {
 		loadingRules.ExplicitPath = kubeConfigPath
-	} else {
-		// Fetch kubeconfig from environment in which this is invoked
-		if _, ok := os.LookupEnv("HOME"); !ok {
-			u, err := user.Current()
-			if err != nil {
-				return nil, fmt.Errorf("could not get current user: %v", err)
-			}
-			loadingRules.Precedence = append(loadingRules.Precedence, path.Join(u.HomeDir, clientcmd.RecommendedHomeDir, clientcmd.RecommendedFileName))
-		}
 	}
 
 	overrides := &clientcmd.ConfigOverrides{}