From 41a99d711af778a177f07402217b85d456b50da1 Mon Sep 17 00:00:00 2001 From: Kubernetes Publisher Date: Sat, 3 Dec 2016 08:16:09 +0000 Subject: [PATCH] published by bot (https://github.com/kubernetes/contrib/tree/master/mungegithub) copied from https://github.com/kubernetes/kubernetes.git, branch master, last commit is 238ffdd0d6d79d610cea2ebe3a03868a197283c8 --- Godeps/Godeps.json | 2 +- pkg/util/strategicpatch/patch.go | 282 +++++++++++++++--- rest/config.go | 15 +- rest/config_test.go | 2 +- rest/transport.go | 6 +- tools/clientcmd/client_config.go | 4 +- tools/record/events_cache.go | 4 +- transport/config.go | 14 +- transport/round_trippers.go | 43 ++- transport/round_trippers_test.go | 56 ++++ vendor/github.com/spf13/pflag/flag.go | 6 +- vendor/github.com/spf13/pflag/string_array.go | 3 +- vendor/github.com/spf13/pflag/string_slice.go | 2 +- 13 files changed, 370 insertions(+), 69 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 9429c714..2f3ea8f0 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -183,7 +183,7 @@ }, { "ImportPath": "github.com/spf13/pflag", - "Rev": "5ccb023bc27df288a957c5e994cd44fd19619465" + "Rev": "c7e63cf4530bcd3ba943729cee0efeff2ebea63f" }, { "ImportPath": "github.com/stretchr/testify/assert", diff --git a/pkg/util/strategicpatch/patch.go b/pkg/util/strategicpatch/patch.go index 56affaa9..0f9b3122 100644 --- a/pkg/util/strategicpatch/patch.go +++ b/pkg/util/strategicpatch/patch.go @@ -21,6 +21,7 @@ import ( "reflect" "sort" + "k8s.io/client-go/discovery" forkedjson "k8s.io/client-go/pkg/third_party/forked/golang/json" "k8s.io/client-go/pkg/util/json" @@ -38,11 +39,20 @@ import ( // Some of the content of this package was borrowed with minor adaptations from // evanphx/json-patch and openshift/origin. +type StrategicMergePatchVersion string + const ( - directiveMarker = "$patch" - deleteDirective = "delete" - replaceDirective = "replace" - mergeDirective = "merge" + directiveMarker = "$patch" + deleteDirective = "delete" + replaceDirective = "replace" + mergeDirective = "merge" + mergePrimitivesListDirective = "mergeprimitiveslist" + + // different versions of StrategicMergePatch + SMPatchVersion_1_0 StrategicMergePatchVersion = "v1.0.0" + SMPatchVersion_1_5 StrategicMergePatchVersion = "v1.5.0" + Unknown StrategicMergePatchVersion = "Unknown" + SMPatchVersionLatest = SMPatchVersion_1_5 ) // IsPreconditionFailed returns true if the provided error indicates @@ -87,6 +97,7 @@ func IsConflict(err error) bool { var errBadJSONDoc = fmt.Errorf("Invalid JSON document") var errNoListOfLists = fmt.Errorf("Lists of lists are not supported") +var errNoElementsInSlice = fmt.Errorf("no elements in any of the given slices") // The following code is adapted from github.com/openshift/origin/pkg/util/jsonmerge. // Instead of defining a Delta that holds an original, a patch and a set of preconditions, @@ -133,15 +144,15 @@ func RequireMetadataKeyUnchanged(key string) PreconditionFunc { } // Deprecated: Use the synonym CreateTwoWayMergePatch, instead. -func CreateStrategicMergePatch(original, modified []byte, dataStruct interface{}) ([]byte, error) { - return CreateTwoWayMergePatch(original, modified, dataStruct) +func CreateStrategicMergePatch(original, modified []byte, dataStruct interface{}, smPatchVersion StrategicMergePatchVersion) ([]byte, error) { + return CreateTwoWayMergePatch(original, modified, dataStruct, smPatchVersion) } // CreateTwoWayMergePatch creates a patch that can be passed to StrategicMergePatch from an original // document and a modified document, which are passed to the method as json encoded content. It will // return a patch that yields the modified document when applied to the original document, or an error // if either of the two documents is invalid. -func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, fns ...PreconditionFunc) ([]byte, error) { +func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, smPatchVersion StrategicMergePatchVersion, fns ...PreconditionFunc) ([]byte, error) { originalMap := map[string]interface{}{} if len(original) > 0 { if err := json.Unmarshal(original, &originalMap); err != nil { @@ -161,7 +172,7 @@ func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, f return nil, err } - patchMap, err := diffMaps(originalMap, modifiedMap, t, false, false) + patchMap, err := diffMaps(originalMap, modifiedMap, t, false, false, smPatchVersion) if err != nil { return nil, err } @@ -177,7 +188,7 @@ func CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}, f } // Returns a (recursive) strategic merge patch that yields modified when applied to original. -func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreChangesAndAdditions, ignoreDeletions bool) (map[string]interface{}, error) { +func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreChangesAndAdditions, ignoreDeletions bool, smPatchVersion StrategicMergePatchVersion) (map[string]interface{}, error) { patch := map[string]interface{}{} if t.Kind() == reflect.Ptr { t = t.Elem() @@ -230,7 +241,7 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreC return nil, err } - patchValue, err := diffMaps(originalValueTyped, modifiedValueTyped, fieldType, ignoreChangesAndAdditions, ignoreDeletions) + patchValue, err := diffMaps(originalValueTyped, modifiedValueTyped, fieldType, ignoreChangesAndAdditions, ignoreDeletions, smPatchVersion) if err != nil { return nil, err } @@ -248,13 +259,25 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreC } if fieldPatchStrategy == mergeDirective { - patchValue, err := diffLists(originalValueTyped, modifiedValueTyped, fieldType.Elem(), fieldPatchMergeKey, ignoreChangesAndAdditions, ignoreDeletions) + patchValue, err := diffLists(originalValueTyped, modifiedValueTyped, fieldType.Elem(), fieldPatchMergeKey, ignoreChangesAndAdditions, ignoreDeletions, smPatchVersion) if err != nil { return nil, err } + if patchValue == nil { + continue + } - if len(patchValue) > 0 { - patch[key] = patchValue + switch typedPatchValue := patchValue.(type) { + case []interface{}: + if len(typedPatchValue) > 0 { + patch[key] = typedPatchValue + } + case map[string]interface{}: + if len(typedPatchValue) > 0 { + patch[key] = typedPatchValue + } + default: + return nil, fmt.Errorf("invalid type of patch: %v", reflect.TypeOf(patchValue)) } continue @@ -284,7 +307,7 @@ func diffMaps(original, modified map[string]interface{}, t reflect.Type, ignoreC // Returns a (recursive) strategic merge patch that yields modified when applied to original, // for a pair of lists with merge semantics. -func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreChangesAndAdditions, ignoreDeletions bool) ([]interface{}, error) { +func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreChangesAndAdditions, ignoreDeletions bool, smPatchVersion StrategicMergePatchVersion) (interface{}, error) { if len(original) == 0 { if len(modified) == 0 || ignoreChangesAndAdditions { return nil, nil @@ -298,12 +321,14 @@ func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string return nil, err } - var patch []interface{} + var patch interface{} if elementType.Kind() == reflect.Map { - patch, err = diffListsOfMaps(original, modified, t, mergeKey, ignoreChangesAndAdditions, ignoreDeletions) - } else if !ignoreChangesAndAdditions { - patch, err = diffListsOfScalars(original, modified) + patch, err = diffListsOfMaps(original, modified, t, mergeKey, ignoreChangesAndAdditions, ignoreDeletions, smPatchVersion) + } else if elementType.Kind() == reflect.Slice { + err = errNoListOfLists + } else { + patch, err = diffListsOfScalars(original, modified, ignoreChangesAndAdditions, ignoreDeletions, smPatchVersion) } if err != nil { @@ -315,8 +340,23 @@ func diffLists(original, modified []interface{}, t reflect.Type, mergeKey string // Returns a (recursive) strategic merge patch that yields modified when applied to original, // for a pair of lists of scalars with merge semantics. -func diffListsOfScalars(original, modified []interface{}) ([]interface{}, error) { - if len(modified) == 0 { +func diffListsOfScalars(original, modified []interface{}, ignoreChangesAndAdditions, ignoreDeletions bool, smPatchVersion StrategicMergePatchVersion) (interface{}, error) { + originalScalars := uniqifyAndSortScalars(original) + modifiedScalars := uniqifyAndSortScalars(modified) + + switch smPatchVersion { + case SMPatchVersion_1_5: + return diffListsOfScalarsIntoMap(originalScalars, modifiedScalars, ignoreChangesAndAdditions, ignoreDeletions) + case SMPatchVersion_1_0: + return diffListsOfScalarsIntoSlice(originalScalars, modifiedScalars, ignoreChangesAndAdditions, ignoreDeletions) + default: + return nil, fmt.Errorf("Unknown StrategicMergePatchVersion: %v", smPatchVersion) + } +} + +func diffListsOfScalarsIntoSlice(originalScalars, modifiedScalars []interface{}, ignoreChangesAndAdditions, ignoreDeletions bool) ([]interface{}, error) { + originalIndex, modifiedIndex := 0, 0 + if len(modifiedScalars) == 0 { // There is no need to check the length of original because there is no way to create // a patch that deletes a scalar from a list of scalars with merge semantics. return nil, nil @@ -324,18 +364,14 @@ func diffListsOfScalars(original, modified []interface{}) ([]interface{}, error) patch := []interface{}{} - originalScalars := uniqifyAndSortScalars(original) - modifiedScalars := uniqifyAndSortScalars(modified) - originalIndex, modifiedIndex := 0, 0 - loopB: for ; modifiedIndex < len(modifiedScalars); modifiedIndex++ { for ; originalIndex < len(originalScalars); originalIndex++ { - originalString := fmt.Sprintf("%v", original[originalIndex]) - modifiedString := fmt.Sprintf("%v", modified[modifiedIndex]) + originalString := fmt.Sprintf("%v", originalScalars[originalIndex]) + modifiedString := fmt.Sprintf("%v", modifiedScalars[modifiedIndex]) if originalString >= modifiedString { if originalString != modifiedString { - patch = append(patch, modified[modifiedIndex]) + patch = append(patch, modifiedScalars[modifiedIndex]) } continue loopB @@ -349,7 +385,57 @@ loopB: // Add any remaining items found only in modified for ; modifiedIndex < len(modifiedScalars); modifiedIndex++ { - patch = append(patch, modified[modifiedIndex]) + patch = append(patch, modifiedScalars[modifiedIndex]) + } + + return patch, nil +} + +func diffListsOfScalarsIntoMap(originalScalars, modifiedScalars []interface{}, ignoreChangesAndAdditions, ignoreDeletions bool) (map[string]interface{}, error) { + originalIndex, modifiedIndex := 0, 0 + patch := map[string]interface{}{} + patch[directiveMarker] = mergePrimitivesListDirective + + for originalIndex < len(originalScalars) && modifiedIndex < len(modifiedScalars) { + originalString := fmt.Sprintf("%v", originalScalars[originalIndex]) + modifiedString := fmt.Sprintf("%v", modifiedScalars[modifiedIndex]) + + // objects are identical + if originalString == modifiedString { + originalIndex++ + modifiedIndex++ + continue + } + + if originalString > modifiedString { + if !ignoreChangesAndAdditions { + modifiedValue := modifiedScalars[modifiedIndex] + patch[modifiedString] = modifiedValue + } + modifiedIndex++ + } else { + if !ignoreDeletions { + patch[originalString] = nil + } + originalIndex++ + } + } + + // Delete any remaining items found only in original + if !ignoreDeletions { + for ; originalIndex < len(originalScalars); originalIndex++ { + originalString := fmt.Sprintf("%v", originalScalars[originalIndex]) + patch[originalString] = nil + } + } + + // Add any remaining items found only in modified + if !ignoreChangesAndAdditions { + for ; modifiedIndex < len(modifiedScalars); modifiedIndex++ { + modifiedString := fmt.Sprintf("%v", modifiedScalars[modifiedIndex]) + modifiedValue := modifiedScalars[modifiedIndex] + patch[modifiedString] = modifiedValue + } } return patch, nil @@ -360,7 +446,7 @@ var errBadArgTypeFmt = "expected a %s, but received a %s" // Returns a (recursive) strategic merge patch that yields modified when applied to original, // for a pair of lists of maps with merge semantics. -func diffListsOfMaps(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreChangesAndAdditions, ignoreDeletions bool) ([]interface{}, error) { +func diffListsOfMaps(original, modified []interface{}, t reflect.Type, mergeKey string, ignoreChangesAndAdditions, ignoreDeletions bool, smPatchVersion StrategicMergePatchVersion) ([]interface{}, error) { patch := make([]interface{}, 0) originalSorted, err := sortMergeListsByNameArray(original, t, mergeKey, false) @@ -406,7 +492,7 @@ loopB: if originalString >= modifiedString { if originalString == modifiedString { // Merge key values are equal, so recurse - patchValue, err := diffMaps(originalMap, modifiedMap, t, ignoreChangesAndAdditions, ignoreDeletions) + patchValue, err := diffMaps(originalMap, modifiedMap, t, ignoreChangesAndAdditions, ignoreDeletions, smPatchVersion) if err != nil { return nil, err } @@ -542,7 +628,15 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin return map[string]interface{}{}, nil } - return nil, fmt.Errorf(errBadPatchTypeFmt, v, patch) + if v == mergePrimitivesListDirective { + // delete the directiveMarker's key-value pair to avoid delta map and delete map + // overlaping with each other when calculating a ThreeWayDiff for list of Primitives. + // Otherwise, the overlaping will cause it calling LookupPatchMetadata() which will + // return an error since the metadata shows it's a slice but it is actually a map. + delete(original, directiveMarker) + } else { + return nil, fmt.Errorf(errBadPatchTypeFmt, v, patch) + } } // nil is an accepted value for original to simplify logic in other places. @@ -578,7 +672,9 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin // If they're both maps or lists, recurse into the value. originalType := reflect.TypeOf(original[k]) patchType := reflect.TypeOf(patchV) - if originalType == patchType { + // check if we are trying to merge a slice with a map for list of primitives + isMergeSliceOfPrimitivesWithAPatchMap := originalType != nil && patchType != nil && originalType.Kind() == reflect.Slice && patchType.Kind() == reflect.Map + if originalType == patchType || isMergeSliceOfPrimitivesWithAPatchMap { // First find the fieldPatchStrategy and fieldPatchMergeKey. fieldType, fieldPatchStrategy, fieldPatchMergeKey, err := forkedjson.LookupPatchMetadata(t, k) if err != nil { @@ -600,9 +696,8 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin if originalType.Kind() == reflect.Slice && fieldPatchStrategy == mergeDirective { elemType := fieldType.Elem() typedOriginal := original[k].([]interface{}) - typedPatch := patchV.([]interface{}) var err error - original[k], err = mergeSlice(typedOriginal, typedPatch, elemType, fieldPatchMergeKey) + original[k], err = mergeSlice(typedOriginal, patchV, elemType, fieldPatchMergeKey) if err != nil { return nil, err } @@ -623,13 +718,34 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin // Merge two slices together. Note: This may modify both the original slice and // the patch because getting a deep copy of a slice in golang is highly // non-trivial. -func mergeSlice(original, patch []interface{}, elemType reflect.Type, mergeKey string) ([]interface{}, error) { - if len(original) == 0 && len(patch) == 0 { +// The patch could be a map[string]interface{} representing a slice of primitives. +// If the patch map doesn't has the specific directiveMarker (mergePrimitivesListDirective), +// it returns an error. Please check patch_test.go and find the test case named +// "merge lists of scalars for list of primitives" to see what the patch looks like. +// Patch is still []interface{} for all the other types. +func mergeSlice(original []interface{}, patch interface{}, elemType reflect.Type, mergeKey string) ([]interface{}, error) { + t, err := sliceElementType(original) + if err != nil && err != errNoElementsInSlice { + return nil, err + } + + if patchMap, ok := patch.(map[string]interface{}); ok { + // We try to merge the original slice with a patch map only when the map has + // a specific directiveMarker. Otherwise, this patch will be treated as invalid. + if directiveValue, ok := patchMap[directiveMarker]; ok && directiveValue == mergePrimitivesListDirective { + return mergeSliceOfScalarsWithPatchMap(original, patchMap) + } else { + return nil, fmt.Errorf("Unable to merge a slice with an invalid map") + } + } + + typedPatch := patch.([]interface{}) + if len(original) == 0 && len(typedPatch) == 0 { return original, nil } // All the values must be of the same type, but not a list. - t, err := sliceElementType(original, patch) + t, err = sliceElementType(original, typedPatch) if err != nil { return nil, err } @@ -638,7 +754,7 @@ func mergeSlice(original, patch []interface{}, elemType reflect.Type, mergeKey s if t.Kind() != reflect.Map { // Maybe in the future add a "concat" mode that doesn't // uniqify. - both := append(original, patch...) + both := append(original, typedPatch...) return uniqifyScalars(both), nil } @@ -649,7 +765,7 @@ func mergeSlice(original, patch []interface{}, elemType reflect.Type, mergeKey s // First look for any special $patch elements. patchWithoutSpecialElements := []interface{}{} replace := false - for _, v := range patch { + for _, v := range typedPatch { typedV := v.(map[string]interface{}) patchType, ok := typedV[directiveMarker] if ok { @@ -685,10 +801,10 @@ func mergeSlice(original, patch []interface{}, elemType reflect.Type, mergeKey s return patchWithoutSpecialElements, nil } - patch = patchWithoutSpecialElements + typedPatch = patchWithoutSpecialElements // Merge patch into original. - for _, v := range patch { + for _, v := range typedPatch { // Because earlier we confirmed that all the elements are maps. typedV := v.(map[string]interface{}) mergeValue, ok := typedV[mergeKey] @@ -721,6 +837,36 @@ func mergeSlice(original, patch []interface{}, elemType reflect.Type, mergeKey s return original, nil } +// mergeSliceOfScalarsWithPatchMap merges the original slice with a patch map and +// returns an uniqified and sorted slice of primitives. +// The patch map must have the specific directiveMarker (mergePrimitivesListDirective). +func mergeSliceOfScalarsWithPatchMap(original []interface{}, patch map[string]interface{}) ([]interface{}, error) { + // make sure the patch has the specific directiveMarker () + if directiveValue, ok := patch[directiveMarker]; ok && directiveValue != mergePrimitivesListDirective { + return nil, fmt.Errorf("Unable to merge a slice with an invalid map") + } + delete(patch, directiveMarker) + output := make([]interface{}, 0, len(original)+len(patch)) + for _, value := range original { + valueString := fmt.Sprintf("%v", value) + if v, ok := patch[valueString]; ok { + if v != nil { + output = append(output, v) + } + delete(patch, valueString) + } else { + output = append(output, value) + } + } + for _, value := range patch { + if value != nil { + output = append(output, value) + } + // No action required to delete items that missing from the original slice. + } + return uniqifyAndSortScalars(output), nil +} + // This method no longer panics if any element of the slice is not a map. func findMapInSliceBasedOnKeyValue(m []interface{}, key string, value interface{}) (map[string]interface{}, int, bool, error) { for k, v := range m { @@ -946,7 +1092,7 @@ func sliceElementType(slices ...[]interface{}) (reflect.Type, error) { } if prevType == nil { - return nil, fmt.Errorf("no elements in any of the given slices") + return nil, errNoElementsInSlice } return prevType, nil @@ -1035,6 +1181,10 @@ func mergingMapFieldsHaveConflicts( if leftMarker != rightMarker { return true, nil } + + if leftMarker == mergePrimitivesListDirective && rightMarker == mergePrimitivesListDirective { + return false, nil + } } // Check the individual keys. @@ -1057,12 +1207,29 @@ func mergingMapFieldsHaveConflicts( } func mapsHaveConflicts(typedLeft, typedRight map[string]interface{}, structType reflect.Type) (bool, error) { + isForListOfPrimitives := false + if leftDirective, ok := typedLeft[directiveMarker]; ok { + if rightDirective, ok := typedRight[directiveMarker]; ok { + if leftDirective == mergePrimitivesListDirective && rightDirective == rightDirective { + isForListOfPrimitives = true + } + } + } for key, leftValue := range typedLeft { if key != directiveMarker { if rightValue, ok := typedRight[key]; ok { - fieldType, fieldPatchStrategy, fieldPatchMergeKey, err := forkedjson.LookupPatchMetadata(structType, key) - if err != nil { - return true, err + var fieldType reflect.Type + var fieldPatchStrategy, fieldPatchMergeKey string + var err error + if isForListOfPrimitives { + fieldType = reflect.TypeOf(leftValue) + fieldPatchStrategy = "" + fieldPatchMergeKey = "" + } else { + fieldType, fieldPatchStrategy, fieldPatchMergeKey, err = forkedjson.LookupPatchMetadata(structType, key) + if err != nil { + return true, err + } } if hasConflicts, err := mergingMapFieldsHaveConflicts(leftValue, rightValue, @@ -1172,7 +1339,7 @@ func mapsOfMapsHaveConflicts(typedLeft, typedRight map[string]interface{}, struc // than from original to current. In other words, a conflict occurs if modified changes any key // in a way that is different from how it is changed in current (e.g., deleting it, changing its // value). -func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct interface{}, overwrite bool, fns ...PreconditionFunc) ([]byte, error) { +func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct interface{}, overwrite bool, smPatchVersion StrategicMergePatchVersion, fns ...PreconditionFunc) ([]byte, error) { originalMap := map[string]interface{}{} if len(original) > 0 { if err := json.Unmarshal(original, &originalMap); err != nil { @@ -1203,12 +1370,12 @@ func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct int // from original to modified. To find it, we compute deletions, which are the deletions from // original to modified, and delta, which is the difference from current to modified without // deletions, and then apply delta to deletions as a patch, which should be strictly additive. - deltaMap, err := diffMaps(currentMap, modifiedMap, t, false, true) + deltaMap, err := diffMaps(currentMap, modifiedMap, t, false, true, smPatchVersion) if err != nil { return nil, err } - deletionsMap, err := diffMaps(originalMap, modifiedMap, t, true, false) + deletionsMap, err := diffMaps(originalMap, modifiedMap, t, true, false, smPatchVersion) if err != nil { return nil, err } @@ -1228,7 +1395,7 @@ func CreateThreeWayMergePatch(original, modified, current []byte, dataStruct int // If overwrite is false, and the patch contains any keys that were changed differently, // then return a conflict error. if !overwrite { - changedMap, err := diffMaps(originalMap, currentMap, t, false, false) + changedMap, err := diffMaps(originalMap, currentMap, t, false, false, smPatchVersion) if err != nil { return nil, err } @@ -1263,3 +1430,20 @@ func toYAML(v interface{}) (string, error) { return string(y), nil } + +// GetServerSupportedSMPatchVersion takes a discoveryClient, +// returns the max StrategicMergePatch version supported +func GetServerSupportedSMPatchVersion(discoveryClient discovery.DiscoveryInterface) (StrategicMergePatchVersion, error) { + serverVersion, err := discoveryClient.ServerVersion() + if err != nil { + return Unknown, err + } + serverGitVersion := serverVersion.GitVersion + if serverGitVersion >= string(SMPatchVersion_1_5) { + return SMPatchVersion_1_5, nil + } + if serverGitVersion >= string(SMPatchVersion_1_0) { + return SMPatchVersion_1_0, nil + } + return Unknown, fmt.Errorf("The version is too old: %v\n", serverVersion) +} diff --git a/rest/config.go b/rest/config.go index 6e316d12..bbd9fb47 100644 --- a/rest/config.go +++ b/rest/config.go @@ -71,8 +71,8 @@ type Config struct { // TODO: demonstrate an OAuth2 compatible client. BearerToken string - // Impersonate is the username that this RESTClient will impersonate - Impersonate string + // Impersonate is the configuration that RESTClient will use for impersonation. + Impersonate ImpersonationConfig // Server requires plugin-specified authentication. AuthProvider *clientcmdapi.AuthProviderConfig @@ -119,6 +119,17 @@ type Config struct { // Version string } +// ImpersonationConfig has all the available impersonation options +type ImpersonationConfig struct { + // UserName is the username to impersonate on each request. + UserName string + // Groups are the groups to impersonate on each request. + Groups []string + // Extra is a free-form field which can be used to link some authentication information + // to authorization information. This field allows you to impersonate it. + Extra map[string][]string +} + // TLSClientConfig contains settings to enable transport layer security type TLSClientConfig struct { // Server requires TLS client certificate authentication diff --git a/rest/config_test.go b/rest/config_test.go index c4e62a55..36a45216 100644 --- a/rest/config_test.go +++ b/rest/config_test.go @@ -205,7 +205,7 @@ func TestAnonymousConfig(t *testing.T) { // this is the list of known security related fields, add to this list if a new field // is added to Config, update AnonymousClientConfig to preserve the field otherwise. - expected.Impersonate = "" + expected.Impersonate = ImpersonationConfig{} expected.BearerToken = "" expected.Username = "" expected.Password = "" diff --git a/rest/transport.go b/rest/transport.go index 0672fb46..9841f8cc 100644 --- a/rest/transport.go +++ b/rest/transport.go @@ -89,6 +89,10 @@ func (c *Config) TransportConfig() (*transport.Config, error) { Username: c.Username, Password: c.Password, BearerToken: c.BearerToken, - Impersonate: c.Impersonate, + Impersonate: transport.ImpersonationConfig{ + UserName: c.Impersonate.UserName, + Groups: c.Impersonate.Groups, + Extra: c.Impersonate.Extra, + }, }, nil } diff --git a/tools/clientcmd/client_config.go b/tools/clientcmd/client_config.go index 61bda9f6..b1d2ef23 100644 --- a/tools/clientcmd/client_config.go +++ b/tools/clientcmd/client_config.go @@ -144,7 +144,7 @@ func (config *DirectClientConfig) ClientConfig() (*rest.Config, error) { clientConfig.Host = u.String() } if len(configAuthInfo.Impersonate) > 0 { - clientConfig.Impersonate = configAuthInfo.Impersonate + clientConfig.Impersonate = rest.ImpersonationConfig{UserName: configAuthInfo.Impersonate} } // only try to read the auth information if we are secure @@ -215,7 +215,7 @@ func getUserIdentificationPartialConfig(configAuthInfo clientcmdapi.AuthInfo, fa mergedConfig.BearerToken = string(tokenBytes) } if len(configAuthInfo.Impersonate) > 0 { - mergedConfig.Impersonate = configAuthInfo.Impersonate + mergedConfig.Impersonate = rest.ImpersonationConfig{UserName: configAuthInfo.Impersonate} } if len(configAuthInfo.ClientCertificate) > 0 || len(configAuthInfo.ClientCertificateData) > 0 { mergedConfig.CertFile = configAuthInfo.ClientCertificate diff --git a/tools/record/events_cache.go b/tools/record/events_cache.go index 6e68a220..500ad9a3 100644 --- a/tools/record/events_cache.go +++ b/tools/record/events_cache.go @@ -244,7 +244,9 @@ func (e *eventLogger) eventObserve(newEvent *v1.Event) (*v1.Event, []byte, error newData, _ := json.Marshal(event) oldData, _ := json.Marshal(eventCopy2) - patch, err = strategicpatch.CreateStrategicMergePatch(oldData, newData, event) + // TODO: need to figure out if we need to let eventObserve() use the new behavior of StrategicMergePatch. + // Currently default to old behavior now. Ref: issue #35936 + patch, err = strategicpatch.CreateStrategicMergePatch(oldData, newData, event, strategicpatch.SMPatchVersion_1_0) } // record our new observation diff --git a/transport/config.go b/transport/config.go index 6e5c68a3..eaad99fb 100644 --- a/transport/config.go +++ b/transport/config.go @@ -34,8 +34,8 @@ type Config struct { // Bearer token for authentication BearerToken string - // Impersonate is the username that this Config will impersonate - Impersonate string + // Impersonate is the config that this Config will impersonate using + Impersonate ImpersonationConfig // Transport may be used for custom HTTP behavior. This attribute may // not be specified with the TLS client certificate options. Use @@ -50,6 +50,16 @@ type Config struct { WrapTransport func(rt http.RoundTripper) http.RoundTripper } +// ImpersonationConfig has all the available impersonation options +type ImpersonationConfig struct { + // UserName matches user.Info.GetName() + UserName string + // Groups matches user.Info.GetGroups() + Groups []string + // Extra matches user.Info.GetExtra() + Extra map[string][]string +} + // HasCA returns whether the configuration has a certificate authority or not. func (c *Config) HasCA() bool { return len(c.TLS.CAData) > 0 || len(c.TLS.CAFile) > 0 diff --git a/transport/round_trippers.go b/transport/round_trippers.go index aadf0cbf..3188d062 100644 --- a/transport/round_trippers.go +++ b/transport/round_trippers.go @@ -48,7 +48,9 @@ func HTTPWrappersForConfig(config *Config, rt http.RoundTripper) (http.RoundTrip if len(config.UserAgent) > 0 { rt = NewUserAgentRoundTripper(config.UserAgent, rt) } - if len(config.Impersonate) > 0 { + if len(config.Impersonate.UserName) > 0 || + len(config.Impersonate.Groups) > 0 || + len(config.Impersonate.Extra) > 0 { rt = NewImpersonatingRoundTripper(config.Impersonate, rt) } return rt, nil @@ -133,22 +135,53 @@ func (rt *basicAuthRoundTripper) CancelRequest(req *http.Request) { func (rt *basicAuthRoundTripper) WrappedRoundTripper() http.RoundTripper { return rt.rt } +// These correspond to the headers used in pkg/apis/authentication. We don't want the package dependency, +// but you must not change the values. +const ( + // ImpersonateUserHeader is used to impersonate a particular user during an API server request + ImpersonateUserHeader = "Impersonate-User" + + // ImpersonateGroupHeader is used to impersonate a particular group during an API server request. + // It can be repeated multiplied times for multiple groups. + ImpersonateGroupHeader = "Impersonate-Group" + + // ImpersonateUserExtraHeaderPrefix is a prefix for a header used to impersonate an entry in the + // extra map[string][]string for user.Info. The key for the `extra` map is suffix. + // The same key can be repeated multiple times to have multiple elements in the slice under a single key. + // For instance: + // Impersonate-Extra-Foo: one + // Impersonate-Extra-Foo: two + // results in extra["Foo"] = []string{"one", "two"} + ImpersonateUserExtraHeaderPrefix = "Impersonate-Extra-" +) + type impersonatingRoundTripper struct { - impersonate string + impersonate ImpersonationConfig delegate http.RoundTripper } // NewImpersonatingRoundTripper will add an Act-As header to a request unless it has already been set. -func NewImpersonatingRoundTripper(impersonate string, delegate http.RoundTripper) http.RoundTripper { +func NewImpersonatingRoundTripper(impersonate ImpersonationConfig, delegate http.RoundTripper) http.RoundTripper { return &impersonatingRoundTripper{impersonate, delegate} } func (rt *impersonatingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - if len(req.Header.Get("Impersonate-User")) != 0 { + // use the user header as marker for the rest. + if len(req.Header.Get(ImpersonateUserHeader)) != 0 { return rt.delegate.RoundTrip(req) } req = cloneRequest(req) - req.Header.Set("Impersonate-User", rt.impersonate) + req.Header.Set(ImpersonateUserHeader, rt.impersonate.UserName) + + for _, group := range rt.impersonate.Groups { + req.Header.Add(ImpersonateGroupHeader, group) + } + for k, vv := range rt.impersonate.Extra { + for _, v := range vv { + req.Header.Add(ImpersonateUserExtraHeaderPrefix+k, v) + } + } + return rt.delegate.RoundTrip(req) } diff --git a/transport/round_trippers_test.go b/transport/round_trippers_test.go index 78af9a59..85ea12e0 100644 --- a/transport/round_trippers_test.go +++ b/transport/round_trippers_test.go @@ -18,6 +18,7 @@ package transport import ( "net/http" + "reflect" "testing" ) @@ -99,3 +100,58 @@ func TestUserAgentRoundTripper(t *testing.T) { t.Errorf("unexpected user agent header: %#v", rt.Request) } } + +func TestImpersonationRoundTripper(t *testing.T) { + tcs := []struct { + name string + impersonationConfig ImpersonationConfig + expected map[string][]string + }{ + { + name: "all", + impersonationConfig: ImpersonationConfig{ + UserName: "user", + Groups: []string{"one", "two"}, + Extra: map[string][]string{ + "first": {"A", "a"}, + "second": {"B", "b"}, + }, + }, + expected: map[string][]string{ + ImpersonateUserHeader: {"user"}, + ImpersonateGroupHeader: {"one", "two"}, + ImpersonateUserExtraHeaderPrefix + "First": {"A", "a"}, + ImpersonateUserExtraHeaderPrefix + "Second": {"B", "b"}, + }, + }, + } + + for _, tc := range tcs { + rt := &testRoundTripper{} + req := &http.Request{ + Header: make(http.Header), + } + NewImpersonatingRoundTripper(tc.impersonationConfig, rt).RoundTrip(req) + + for k, v := range rt.Request.Header { + expected, ok := tc.expected[k] + if !ok { + t.Errorf("%v missing %v=%v", tc.name, k, v) + continue + } + if !reflect.DeepEqual(expected, v) { + t.Errorf("%v expected %v: %v, got %v", tc.name, k, expected, v) + } + } + for k, v := range tc.expected { + expected, ok := rt.Request.Header[k] + if !ok { + t.Errorf("%v missing %v=%v", tc.name, k, v) + continue + } + if !reflect.DeepEqual(expected, v) { + t.Errorf("%v expected %v: %v, got %v", tc.name, k, expected, v) + } + } + } +} diff --git a/vendor/github.com/spf13/pflag/flag.go b/vendor/github.com/spf13/pflag/flag.go index fa815642..b0b0d46f 100644 --- a/vendor/github.com/spf13/pflag/flag.go +++ b/vendor/github.com/spf13/pflag/flag.go @@ -416,7 +416,7 @@ func Set(name, value string) error { // otherwise, the default values of all defined flags in the set. func (f *FlagSet) PrintDefaults() { usages := f.FlagUsages() - fmt.Fprint(f.out(), usages) + fmt.Fprintf(f.out(), "%s", usages) } // defaultIsZeroValue returns true if the default value for this flag represents @@ -514,7 +514,7 @@ func (f *FlagSet) FlagUsages() string { if len(flag.NoOptDefVal) > 0 { switch flag.Value.Type() { case "string": - line += fmt.Sprintf("[=\"%s\"]", flag.NoOptDefVal) + line += fmt.Sprintf("[=%q]", flag.NoOptDefVal) case "bool": if flag.NoOptDefVal != "true" { line += fmt.Sprintf("[=%s]", flag.NoOptDefVal) @@ -534,7 +534,7 @@ func (f *FlagSet) FlagUsages() string { line += usage if !flag.defaultIsZeroValue() { if flag.Value.Type() == "string" { - line += fmt.Sprintf(" (default \"%s\")", flag.DefValue) + line += fmt.Sprintf(" (default %q)", flag.DefValue) } else { line += fmt.Sprintf(" (default %s)", flag.DefValue) } diff --git a/vendor/github.com/spf13/pflag/string_array.go b/vendor/github.com/spf13/pflag/string_array.go index 93b4e432..f320f2ec 100644 --- a/vendor/github.com/spf13/pflag/string_array.go +++ b/vendor/github.com/spf13/pflag/string_array.go @@ -2,6 +2,7 @@ package pflag import ( "fmt" + "strings" ) var _ = fmt.Fprint @@ -39,7 +40,7 @@ func (s *stringArrayValue) String() string { } func stringArrayConv(sval string) (interface{}, error) { - sval = sval[1 : len(sval)-1] + sval = strings.Trim(sval, "[]") // An empty string would cause a array with one (empty) string if len(sval) == 0 { return []string{}, nil diff --git a/vendor/github.com/spf13/pflag/string_slice.go b/vendor/github.com/spf13/pflag/string_slice.go index 7829cfaf..51e3c5d2 100644 --- a/vendor/github.com/spf13/pflag/string_slice.go +++ b/vendor/github.com/spf13/pflag/string_slice.go @@ -66,7 +66,7 @@ func (s *stringSliceValue) String() string { } func stringSliceConv(sval string) (interface{}, error) { - sval = sval[1 : len(sval)-1] + sval = strings.Trim(sval, "[]") // An empty string would cause a slice with one (empty) string if len(sval) == 0 { return []string{}, nil