-
Notifications
You must be signed in to change notification settings - Fork 71
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
Updates & Refactors for k8s 1.32 #644
Conversation
} | ||
|
||
// Need to verify encrypted is actually different... | ||
if !bytes.Equal(resourceBytes, maybeEncrypted) { |
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.
Because we can do this check here...
pkg/resourcesets/collector.go
Outdated
@@ -436,17 +436,21 @@ func writeToBackup(ctx context.Context, resource map[string]interface{}, backupP | |||
if err != nil { | |||
return fmt.Errorf("error converting resource to JSON: %v", err) | |||
} | |||
if transformer != nil && !util.IsDefaultEncryptionTransformer(transformer) { |
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.
...it technically does similar to what this intended to do, just after transformer.TransformToStorage
is called instead of before. Obviously before was better, but no longer an option (for) now.
|
||
resourceBytes, err = json.Marshal(encrypted) | ||
// Since k8s 1.32 we cannot verify a transformer must be run, so it's always run now. | ||
maybeEncrypted, err := transformer.TransformToStorage(ctx, resourceBytes, value.DefaultContext(additionalAuthenticatedData)) |
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.
I think that because of those factors, this is a new change in dataflow/processing that non-encrypted restores used to skip, but will now run all the time. So ultimately this likely doesn't change too much at the end of the day, but is technically a new area that an error could come from.
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.
We should bump
- v1.31.4-k3s1 |
The matrix k3s versions here to 1.30.X & 1.32.X
Signed-off-by: Alexandre Lamarre <[email protected]>
Part of rancher/rancher#48832 |
Blocked by: #645
I will submit a version of this based of
main
after above is merged. This will ensure higher test coverage before the breaking upstream change is accounted for.