Skip to content

Commit

Permalink
chore: remove undeeded metadata when applying k8s objects (#9267)
Browse files Browse the repository at this point in the history
### What does this PR do?

* Removes undeeded metadata objects which are not applicable / not
  needed when applying a patch. This includes resourceVersion, UID,
  selfLink and creationTimestamp. These are automatically created when
  applying a resource if undefined / blank.
* This change is needed as Kubernetes javascript client added
  serialization / checks with regards to creationTimestamps as well as
  other metadata objects from PR: kubernetes-client/javascript#1695

### Screenshot / video of UI

<!-- If this PR is changing UI, please include
screenshots or screencasts showing the difference -->

### What issues does this PR fix or reference?

<!-- Include any related issues from Podman Desktop
repository (or from another issue tracker). -->

Closes #9260
Closes #9262

### How to test this PR?

<!-- Please explain steps to verify the functionality,
do not forget to provide unit/component tests -->

- [X] Tests are covering the bug fix or the new feature

1. Edit any k8s yaml
2. Press apply
3. Success
4. Edit again
5. Press apply
6. Success

Signed-off-by: Charlie Drage <charlie@charliedrage.com>
  • Loading branch information
cdrage authored Oct 8, 2024
1 parent 645fc5b commit 5b19313
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 0 deletions.
100 changes: 100 additions & 0 deletions packages/main/src/plugin/kubernetes/kubernetes-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2407,3 +2407,103 @@ test('Should throw an error if unable to restart controlled pod', async () => {

await expect(client.restartPod('test-pod')).rejects.toThrow('unable to restart controlled pod');
});

test('test sync resources was called', async () => {
const client = createTestClient('default');
const context = 'test-context';
const namespace = 'default';
const manifests: KubernetesObject[] = [
{
apiVersion: 'v1',
kind: 'Pod',
metadata: {
name: 'test-pod',
},
},
];

const mockedPatch = vi.fn();

makeApiClientMock.mockReturnValue({
read: vi.fn(),
create: vi.fn(),
patch: mockedPatch,
});

// Call the syncResources method with 'create' action
await client.syncResources(context, manifests, 'apply', namespace);

// Expect patch method to have been called
expect(mockedPatch).toHaveBeenCalled();

// We expect it to have been called with the same object, but with a few "extra" fiels such as last-applied-configuration
expect(mockedPatch).toHaveBeenCalledWith(
{
apiVersion: 'v1',
kind: 'Pod',
metadata: {
annotations: {
// eslint-disable-next-line no-useless-escape
'kubectl.kubernetes.io/last-applied-configuration': `{\"apiVersion\":\"v1\",\"kind\":\"Pod\",\"metadata\":{\"name\":\"test-pod\",\"annotations\":{}}}`,
},
name: 'test-pod',
namespace: 'default',
},
},
undefined,
undefined,
'podman-desktop',
);
});

test('test sync resources was called with no resourceVersion, uid, selfLink, or creationTimestamp being passed through', async () => {
const client = createTestClient('default');
const context = 'test-context';
const namespace = 'default';
const manifests: KubernetesObject[] = [
{
apiVersion: 'v1',
kind: 'Pod',
metadata: {
name: 'test-pod',
resourceVersion: '123',
uid: 'uid123',
selfLink: '/api/v1/namespaces/default/pods/test-pod',
creationTimestamp: new Date(42),
},
},
];

const mockedPatch = vi.fn();

makeApiClientMock.mockReturnValue({
read: vi.fn(),
create: vi.fn(),
patch: mockedPatch,
});

// Call the syncResources method with 'create' action
await client.syncResources(context, manifests, 'apply', namespace);

// Expect patch method to have been called
expect(mockedPatch).toHaveBeenCalled();

// Expect it to be called with NO resourceVersion, uid, selfLink, or creationTimestamp in the metadata, however, it is okay to have it in 'last-applied-configuration'
expect(mockedPatch).toHaveBeenCalledWith(
{
apiVersion: 'v1',
kind: 'Pod',
metadata: {
annotations: {
// eslint-disable-next-line no-useless-escape
'kubectl.kubernetes.io/last-applied-configuration': `{\"apiVersion\":\"v1\",\"kind\":\"Pod\",\"metadata\":{\"name\":\"test-pod\",\"resourceVersion\":\"123\",\"uid\":\"uid123\",\"selfLink\":\"/api/v1/namespaces/default/pods/test-pod\",\"creationTimestamp\":\"1970-01-01T00:00:00.042Z\",\"annotations\":{}}}`,
},
name: 'test-pod',
namespace: 'default',
},
},
undefined,
undefined,
'podman-desktop',
);
});
11 changes: 11 additions & 0 deletions packages/main/src/plugin/kubernetes/kubernetes-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,17 @@ export class KubernetesClient {
//
// See: https://github.com/kubernetes/kubernetes/issues/97423
if (action === 'apply') {
// When patching a resource, we do not need certain metadata fields to be present such as resourceVersion, uid, selfLink, and creationTimestamp
// these cause conflicts when patching a resource since client.patch will serialize these fields and the server will reject the request
// this change is due to changes on how client.patch / client.create works with the latest serialization changes in:
// https://github.com/kubernetes-client/javascript/pull/1695 with regards to date.
// we also remove resourceVersion so we may apply multiple edits to the same resource without having to entirely retrieve and reload the YAML
// from the server before applying.
delete spec.metadata?.resourceVersion;
delete spec.metadata?.uid;
delete spec.metadata?.selfLink;
delete spec.metadata?.creationTimestamp;

const response = await client.patch(
spec,
undefined /* pretty */,
Expand Down

0 comments on commit 5b19313

Please sign in to comment.