Skip to content

Comments

Komoplane crossplane v2#81

Open
eddiewhho wants to merge 9 commits intokomodorio:mainfrom
eddiewhho:komoplane_crosplane_v2
Open

Komoplane crossplane v2#81
eddiewhho wants to merge 9 commits intokomodorio:mainfrom
eddiewhho:komoplane_crosplane_v2

Conversation

@eddiewhho
Copy link

This pull request introduces several improvements and enhancements to the backend, focusing on Crossplane CRD handling, API route expansion, dependency updates, and developer tooling. The most notable changes include more robust and version-aware handling of Crossplane CRDs, expanded API endpoints for better resource access, and improved test coverage for CRD listing logic.

Crossplane CRD Handling Improvements:

  • Refactored the crdClient to support a version-aware XRD client, improving reliability when resolving plural resource names and handling different Crossplane versions. Added extensive debug logging throughout CRD operations for easier troubleshooting. [1] [2] [3] [4]
  • Enhanced the ExtensionsV1Client to attempt using a v2-compatible XRD client with a fallback to the original implementation, making the client more robust to Crossplane API changes. [1] [2]

API Route Expansion:

  • Added new API endpoints for accessing managed and composite resources by namespace and name, and introduced a /composition/:name endpoint for retrieving individual compositions.

Testing and Quality Assurance:

  • Added comprehensive unit tests for the CRD client’s List method, covering all-namespaces queries, error handling, and invalid JSON scenarios, ensuring correct CRD listing behavior.

Dependency and Tooling Updates:

  • Updated Go version to 1.23.8 and refreshed multiple dependencies for improved compatibility and security.
  • Added .vscode/settings.json for improved developer experience with recommended extensions and workspace settings.
  • Enhanced the Makefile with new targets for backend/frontend tests, coverage, and quality checks, and improved help output.

Minor Improvements:

  • Cleaned up imports and improved organization in backend files. [1] [2] [3]

@undera
Copy link
Collaborator

undera commented Sep 10, 2025

Hey
I saw this, but 80 files review would take a long time. If you see a way to break it down into multiple phases - that would speed up the process.

@undera
Copy link
Collaborator

undera commented Sep 28, 2025

I unblocked the CI run, please take a look at the problems

Copy link
Collaborator

@undera undera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impressive work! A bit hard to review as single PR though.
Please clean up the unintentionally added files.
Also CI lint check fails, please address that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file from Git

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plese remove this file

github.com/crossplane-contrib/provider-kubernetes v0.9.0
github.com/crossplane/crossplane v1.13.0
github.com/crossplane/crossplane-runtime v0.20.0
github.com/crossplane/crossplane v1.21.0-rc.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO depending on RC versions is not good. Is there a stable release to depend on?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have top-level go.mod file. Why do we need to introduce this one?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THe whole folder of coverage should be removed, as it is dynamic and not part of codebase. Please use appropriate git config exclude to avoid committing these dynamic files.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this folder? Can't this test live in some other place?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove/exclude this file

if err != nil {
return err
log.Debugf("Resource not found without namespace, trying common namespaces for %s", ref.Name)
namespacesToTry := []string{"vela-app-dev", "default", "crossplane-system", "upbound-system", "kube-system", "vela-system", "flux-system", "argocd", "argo-cd"}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it perhaps possible to paramaterise this list of namespaces (maybe even as a helm input)?

XRs are likley to be provisioned in tenant namespaces which are impossible to predict, especially with the gradual deprecation of claims

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