-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix continuous diff for Secret stringData field #2511
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,22 +65,35 @@ func ToUnstructured(object metav1.Object) (*unstructured.Unstructured, error) { | |
// This process normalizes semantically-equivalent resources into an identical output, which is important for diffing. | ||
// If the scheme is not defined, then return the original resource. | ||
func Normalize(uns *unstructured.Unstructured) (*unstructured.Unstructured, error) { | ||
var result *unstructured.Unstructured | ||
|
||
if IsCRD(uns) { | ||
return normalizeCRD(uns), nil | ||
result = normalizeCRD(uns) | ||
} else if IsSecret(uns) { | ||
result = normalizeSecret(uns) | ||
} else { | ||
obj, err := FromUnstructured(uns) | ||
// Return the input resource rather than an error if this operation fails. | ||
if err != nil { | ||
return uns, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was it intentional to ignore all errors, e.g. a value parsing error? For example, given |
||
} | ||
result, err = ToUnstructured(obj) | ||
// Return the input resource rather than an error if this operation fails. | ||
if err != nil { | ||
return uns, err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was it intentional to return the |
||
} | ||
} | ||
|
||
obj, err := FromUnstructured(uns) | ||
// Return the input resource rather than an error if this operation fails. | ||
if err != nil { | ||
return uns, nil | ||
} | ||
return ToUnstructured(obj) | ||
// Remove output-only fields | ||
unstructured.RemoveNestedField(result.Object, "metadata", "creationTimestamp") | ||
|
||
return result, nil | ||
} | ||
|
||
// normalizeCRD manually normalizes CRD resources, which require special handling due to the lack of defined conversion | ||
// scheme for CRDs. | ||
func normalizeCRD(uns *unstructured.Unstructured) *unstructured.Unstructured { | ||
contract.Assertf(IsCRD(uns), "normalizeCRD called on a non-CRD resource: %s", uns.GetAPIVersion()) | ||
contract.Assertf(IsCRD(uns), "normalizeCRD called on a non-CRD resource: %s:%s", uns.GetAPIVersion(), uns.GetKind()) | ||
|
||
// .spec.preserveUnknownFields is deprecated, and will be removed by the apiserver on the created resource if the | ||
// value is false. Normalize for diffing by removing this field if present and set to "false". | ||
|
@@ -92,6 +105,37 @@ func normalizeCRD(uns *unstructured.Unstructured) *unstructured.Unstructured { | |
return uns | ||
} | ||
|
||
// normalizeSecret manually normalizes Secret resources, which require special handling due to the apiserver replacing | ||
// the .stringData field with a base64-encoded value in the .data field. | ||
func normalizeSecret(uns *unstructured.Unstructured) *unstructured.Unstructured { | ||
contract.Assertf(IsSecret(uns), "normalizeSecret called on a non-Secret resource: %s:%s", uns.GetAPIVersion(), uns.GetKind()) | ||
|
||
obj, err := FromUnstructured(uns) | ||
if err != nil { | ||
return uns // If the operation fails, just return the original object | ||
} | ||
secret := obj.(*corev1.Secret) | ||
|
||
// See https://github.com/kubernetes/kubernetes/blob/v1.27.4/pkg/apis/core/v1/conversion.go#L406-L414 | ||
// StringData overwrites Data | ||
if len(secret.StringData) > 0 { | ||
if secret.Data == nil { | ||
secret.Data = map[string][]byte{} | ||
} | ||
for k, v := range secret.StringData { | ||
secret.Data[k] = []byte(v) | ||
} | ||
|
||
secret.StringData = nil | ||
} | ||
|
||
updated, err := ToUnstructured(secret) | ||
if err != nil { | ||
return uns // If the operation fails, just return the original object | ||
} | ||
return updated | ||
} | ||
|
||
func PodFromUnstructured(uns *unstructured.Unstructured) (*corev1.Pod, error) { | ||
const expectedAPIVersion = "v1" | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: preference for switch case here, but will follow up in a later PR/discussion to get this fix out ASAP.