Skip to content
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

tests:fix: native type arrays #3636

Closed
wants to merge 1 commit into from

Conversation

acpana
Copy link
Collaborator

@acpana acpana commented Feb 7, 2025

Correctly address terminal array fields (they will look like spec.foo.bar.xyz[])

addresses #3571

Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign cheftako for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@acpana acpana marked this pull request as ready for review February 7, 2025 20:25
@acpana acpana requested review from justinsb and yuwenma February 7, 2025 20:26
@jasonvigil
Copy link
Collaborator

Hmm, this does not appear to work for arrays of objects.

For example, looks like it still throws an error for

[missing_field] crd=workstations.workstations.cnrm.cloud.google.com version=v1beta1: field ".spec.annotations[].value" is not set in unstructured objects

, even though that field is being set in https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/master/pkg/test/resourcefixture/testdata/basic/workstations/workstation/workstation-full/create.yaml

Is there any way to implement a more general fix for arrays of all types? Not just native type arrays. IMO, this does not actually fix the original issue.

@acpana
Copy link
Collaborator Author

acpana commented Feb 7, 2025

For example, looks like it still throws an error for

Good catch! I think we can keep the issue open as we investigate that case.

I think this is a good fix for native types for now and we can follow up for other array cases.

@jingyih
Copy link
Collaborator

jingyih commented Feb 11, 2025

Oh, I just saw this PR. I sent #3651 to unblock my other PRs, and I believe it also fixes the "arrays of objects" issue.

@acpana
Copy link
Collaborator Author

acpana commented Feb 11, 2025

Closing in favor of #3651

Good job Jingyi !!

@acpana acpana closed this Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants