Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions pkg/core/secret/from_kube.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package secret

import (
"fmt"
"reflect"

serrors "github.com/koki/structurederrors"
v1 "k8s.io/api/core/v1"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get rid of the name ("v1") here and in other files. Not needed

)

// NewSecretFromKubeSecret will create a new Secret object with
// the data from a provided kubernetes Secret object
func NewSecretFromKubeSecret(s interface{}) (*Secret, error) {
switch reflect.TypeOf(s) {
case reflect.TypeOf(v1.Secret{}):
obj := s.(v1.Secret)
return fromKubeSecretV1(&obj)
case reflect.TypeOf(&v1.Secret{}):
return fromKubeSecretV1(s.(*v1.Secret))
default:
return nil, fmt.Errorf("unknown Secret version: %s", reflect.TypeOf(s))
}
}

func fromKubeSecretV1(kubeSecret *v1.Secret) (*Secret, error) {
sType, err := convertSecretType(kubeSecret.Type)
if err != nil {
return nil, err
}
s := &Secret{
Name: kubeSecret.Name,
Namespace: kubeSecret.Namespace,
Version: kubeSecret.APIVersion,
Cluster: kubeSecret.ClusterName,
Labels: kubeSecret.Labels,
Annotations: kubeSecret.Annotations,
Data: kubeSecret.Data,
StringData: kubeSecret.StringData,
SecretType: sType,
}

return s, nil
}

// convertSecretType converts from a kubernetes SecretType
func convertSecretType(secret v1.SecretType) (SecretType, error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this in the same vain as other conversion functions (ie fromKubeSecreTypetV1). Name them after the Kubernetes object name and not the mantle one (if there's a difference). Keep in mind that there will likely be multiple conversion functions for multiple API versions, so name functions with that in mind.

if secret == "" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't be valid once secret is an int. Look at other similar impls in pod to see how to handle

Copy link
Author

@VSharapov VSharapov Mar 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it supposed to be valid when an int is passed? Or should we just check against all valid options and do what you did in podtemplate - return nil, error?
Or should we create a SecretTypeUnset and return SecretTypeUnset, error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, gonna remove the confusion that I caused myself with a quick s/secret/sType/g in this function

return "", nil
}
switch secret {
case v1.SecretTypeOpaque:
return SecretTypeOpaque, nil
case v1.SecretTypeServiceAccountToken:
return SecretTypeServiceAccountToken, nil
case v1.SecretTypeDockercfg:
return SecretTypeDockercfg, nil
case v1.SecretTypeDockerConfigJson:
return SecretTypeDockerConfigJson, nil
case v1.SecretTypeBasicAuth:
return SecretTypeBasicAuth, nil
case v1.SecretTypeSSHAuth:
return SecretTypeSSHAuth, nil
case v1.SecretTypeTLS:
return SecretTypeTLS, nil
default:
return "", serrors.InvalidValueErrorf(secret, "unrecognized Secret type")
}
}
61 changes: 61 additions & 0 deletions pkg/core/secret/secret.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package secret

import (
"fmt"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run through gofmt

v1 "k8s.io/api/core/v1"
)

type SecretWrapper struct {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REmove this. Wrappers aren't needed in mantle

Secret Secret `json:"secret,omitempty"`
}

type Secret struct {
Version string `json:"version,omitempty"`
Cluster string `json:"cluster,omitempty"`
Name string `json:"name,omitempty"`
Namespace string `json:"namespace,omitempty"`
Labels map[string]string `json:"labels,omitempty"`
Annotations map[string]string `json:"annotations,omitempty"`
StringData map[string]string `json:"string_data,omitempty"`
Data map[string][]byte `json:"data,omitempty"`
SecretType SecretType `json:"type,omitempty"`
}

type SecretType string
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this an int and all definitions of it using iota


const (
SecretTypeOpaque SecretType = "Opaque"
SecretTypeServiceAccountToken SecretType = "kubernetes.io/service-account-token"
SecretTypeDockercfg SecretType = "kubernetes.io/dockercfg"
SecretTypeDockerConfigJson SecretType = "kubernetes.io/dockerconfigjson"
SecretTypeBasicAuth SecretType = "kubernetes.io/basic-auth"
SecretTypeSSHAuth SecretType = "kubernetes.io/ssh-auth"
SecretTypeTLS SecretType = "kubernetes.io/tls"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the full set of types from kubernetes. SecretTypeBootstrapToken is missing

)

func (s SecretType) ToString() string {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function not needed

return string(s)
}

func CompareSecretTypes(secret1, secret2 interface{}) (bool, error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function shouldn't be needed anymore or if it is, modified to work with secret type being an int. Also seems more like a test function, so move into test file and don't export

s1str, s2str := "", ""
sV1, ok1 := secret1.(v1.SecretType)
if ok1 { s1str = string(sV1) }
sMantle, ok2 := secret1.(SecretType)
if ok2 { s1str = string(sMantle) }
if !ok1 && !ok2 {
return false, fmt.Errorf("%s is not a k8s.v1 secretType nor a mantle SecretType", secret1)
}

sV1, ok1 = secret2.(v1.SecretType)
if ok1 { s2str = string(sV1) }
sMantle, ok2 = secret2.(SecretType)
if ok2 { s2str = string(sMantle) }
if !ok1 && !ok2 {
return false, fmt.Errorf("%s is not a k8s.v1 secretType nor a mantle SecretType", secret2)
}

return s1str == s2str, nil
}


247 changes: 247 additions & 0 deletions pkg/core/secret/secret_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
package secret

import (
"reflect"
"testing"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// map[SecretName]V1SecretName
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented out code

var mappings = map[string]string{
"Name": "Name",
"Namespace": "Namespace",
"Version": "APIVersion",
"Cluster": "ClusterName",
"Labels": "Labels",
"Annotations": "Annotations",
"Data": "Data",
"StringData": "StringData",
"SecretType": "Type",
}

var mapToSecretType = map[v1.SecretType]SecretType{
v1.SecretTypeOpaque: SecretTypeOpaque,
v1.SecretTypeServiceAccountToken: SecretTypeServiceAccountToken,
v1.SecretTypeDockercfg: SecretTypeDockercfg,
v1.SecretTypeDockerConfigJson: SecretTypeDockerConfigJson,
v1.SecretTypeBasicAuth: SecretTypeBasicAuth,
v1.SecretTypeSSHAuth: SecretTypeSSHAuth,
v1.SecretTypeTLS: SecretTypeTLS,
}

func TestCompareSecretTypes(t *testing.T) {
// Compare mantle to v1
for key, val := range mapToSecretType {
out, err := CompareSecretTypes(key, val)
if !out || err != nil {
t.Errorf("Failed TestCompareSecretTypes on %s and %s", key, val)
}
}
// Compare v1 to v1
for key, _ := range mapToSecretType {
out, err := CompareSecretTypes(key, key)
if !out || err != nil {
t.Errorf("Failed TestCompareSecretTypes on %s and %s", key, key)
}
}
// Compare mantle to mantle
for _, val := range mapToSecretType {
out, err := CompareSecretTypes(val, val)
if !out || err != nil {
t.Errorf("Failed TestCompareSecretTypes on %s and %s", val, val)
}
}
// Compare "Opaque" to k8s v1.SecretTypeOpaque
out, err := CompareSecretTypes("Opaque", v1.SecretTypeOpaque)
if err == nil {
t.Errorf("Compare \"Opaque\" to k8s v1.SecretTypeOpaque should have failed but returned %v", out)
}
}


// TestNewSecretFromKubeSecret verifies that NewSecretFromKubeSecret()
// successfully creates a Secret from a kubernetes Secret
func TestNewSecretFromKubeSecret(t *testing.T) {
testcases := []struct {
description string
obj interface{}
}{
{
description: "v1 secret object",
obj: v1.Secret{},
},
{
description: "v1 secret pointer",
obj: &v1.Secret{},
},
}

for _, tc := range testcases {
obj, _ := NewSecretFromKubeSecret(tc.obj)
expectedObj := reflect.TypeOf(&Secret{})
objType := reflect.TypeOf(obj)
if expectedObj != objType {
t.Errorf("expected %s got %s", expectedObj, objType)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put tc description at head of error message (ie "%s: ...")

}
}
}

// TestFromKubeSecretV1 verifies that fromKubeSecretV1() correctly populates
// the v1.Secret{} fields
func TestFromKubeSecretV1(t *testing.T) {
v1S := v1.Secret{
TypeMeta: metav1.TypeMeta{
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "testS",
Namespace: "testNS",
ClusterName: "testCluster",
Labels: map[string]string{"label1": "test1", "label2": "test2"},
Annotations: map[string]string{"ann1": "test1", "ann2": "test2"},
},
StringData: map[string]string{"field1": "data1", "field2": "data2"},
Data: map[string][]byte{"bfield1": []byte("bdata1")},
Type: v1.SecretTypeOpaque,
}

s, _ := fromKubeSecretV1(&v1S)
if s.Name != v1S.Name {
t.Errorf("incorrect name, expected %s got %s", v1S.Name, s.Name)
}

for name, v1Name := range mappings {
value := reflect.ValueOf(s).Elem().FieldByName(name).Interface()
v1Value := reflect.ValueOf(v1S).FieldByName(v1Name).Interface()
//fmt.Printf("(%v, %T)\n", value, value)
//fmt.Printf("(%v, %T)\n", v1Value, v1Value)
if !reflect.DeepEqual(value, v1Value) && name != "SecretType" {
t.Errorf("incorrect %s, expected %s, got %s", name, v1Value, value)
}
if name == "SecretType" {
areEqual, err := CompareSecretTypes(value, v1Value)
if err != nil {
t.Errorf("%s", err)
}
if !areEqual {
t.Errorf("incorrect %s, expected %s, got %s", name, v1Value, value)
}
}
}
}

// TestToKube verifies that ToKube() successfully returns
// a v1.Secret{}
func TestToKube(t *testing.T) {
testcases := []struct {
description string
version string
expectedObj interface{}
}{
{
description: "v1 api version",
version: "v1",
expectedObj: &v1.Secret{},
},
{
description: "empty api version",
version: "",
expectedObj: &v1.Secret{},
},
{
description: "unknown api version",
version: "unknown",
expectedObj: nil,
},
}

for _, tc := range testcases {
s := Secret{
Version: tc.version,
}
kubeObj, err := s.ToKube()
kubeType := reflect.TypeOf(kubeObj)
expectedType := reflect.TypeOf(tc.expectedObj)
if kubeType != expectedType {
t.Errorf("wrong api version, got %s expected %s", kubeType, expectedType)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the tc description at the head of the error message (ie "%s: ...)

}
if tc.expectedObj == nil && err == nil {
t.Errorf("no error returned")
}
}
}

// TestToKubeV1 verifies that toKubev1() correctly populates
// the v1.Secret{} fields
func TestToKubeV1(t *testing.T) {
s := Secret{
Version: "v1",
Name: "testS",
Namespace: "testNS",
Cluster: "testCluster",
Labels: map[string]string{"label1": "test1", "label2": "test2"},
Annotations: map[string]string{"ann1": "test1", "ann2": "test2"},
StringData: map[string]string{"field1": "data1", "field2": "data2"},
Data: map[string][]byte{"bfield1": []byte("bdata1")},
SecretType: SecretTypeOpaque,
}

kubeObj, _ := s.toKubeV1()
for name, v1Name := range mappings {
value := reflect.ValueOf(s).FieldByName(name).Interface()
v1Value := reflect.ValueOf(kubeObj).Elem().FieldByName(v1Name).Interface()
if !reflect.DeepEqual(value, v1Value) && name != "SecretType" {
t.Errorf("incorrect %s, expected %s, got %s", name, v1Value, value)
}
if name == "SecretType" {
areEqual, err := CompareSecretTypes(value, v1Value)
if err != nil {
t.Errorf("%s", err)
}
if !areEqual {
t.Errorf("incorrect %s, expected %s, got %s", name, v1Value, value)
}
}
}
}

// TestConvertSecretType verifies converting to Secret Types from Kubernetes
func TestConvertSecretType(t *testing.T) {
for kubeSecretType, secretType := range mapToSecretType {
gotType, err := convertSecretType(kubeSecretType)
if err != nil {
t.Errorf("convertSecretType() returned an error: %s", err)
}

if gotType != secretType {
t.Errorf("incorrect conversion of %s, expected %s, got %s", kubeSecretType, secretType, gotType)
}
}
}

var mapToKubeSecretType = map[SecretType]v1.SecretType{
SecretTypeOpaque: v1.SecretTypeOpaque,
SecretTypeServiceAccountToken: v1.SecretTypeServiceAccountToken,
SecretTypeDockercfg: v1.SecretTypeDockercfg,
SecretTypeDockerConfigJson: v1.SecretTypeDockerConfigJson,
SecretTypeBasicAuth: v1.SecretTypeBasicAuth,
SecretTypeSSHAuth: v1.SecretTypeSSHAuth,
SecretTypeTLS: v1.SecretTypeTLS,
}

// TestRevertSecretType verifies converting to Kubernetes Secret Types
func TestRevertSecretType(t *testing.T) {
for secretType, KubeSecretType := range mapToKubeSecretType {
gotType, err := revertSecretType(secretType)
if err != nil {
t.Errorf("revertSecretType() returned an error: %s", err)
}

// t.Errorf("gotType: %s\nsecretType: %s\nKubeSecretType: %s", gotType, secretType, KubeSecretType)
if string(gotType) != secretType.ToString() {
t.Errorf("incorrect conversion of %s, expected %s, got %s", secretType, KubeSecretType, gotType)
}
}
}
Loading