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

Extra resources with matchName does not work #172

Closed
irizzant opened this issue Oct 16, 2024 · 14 comments · Fixed by #175 or #176
Closed

Extra resources with matchName does not work #172

irizzant opened this issue Oct 16, 2024 · 14 comments · Fixed by #175 or #176
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@irizzant
Copy link

irizzant commented Oct 16, 2024

What happened?

I have the following Extra Resources defined

_items = [{
    apiVersion = "meta.krm.kcl.dev/v1alpha1"
    kind = "ExtraResources"
    requirements = {
        test = {
            apiVersion = "platform.eng.it/v1alpha1"
            kind = "Project"
            matchName = "example-project"
        }
    }
}]

which matches the following project definition:

apiVersion: platform.eng.it/v1alpha1
kind: Project
metadata:
  name: example-project
  namespace: default
  spec:

I then try to use this resource inside a Composition like this:

name = er?.test[0]?.Resource?.metadata?.name if er?.test else ""

_items += [
        {
            apiVersion = "repo.github.upbound.io/v1alpha1"
            kind = "Repository"
            metadata = _metadata("repository", "app") | {
                labels = {
                    "test" = name
                }
            }
        }
]

But the end result is that the test annotation is empty in my kubernetes cluster:

apiVersion: repo.github.upbound.io/v1alpha1
kind: Repository
metadata:
  labels:
    test: ""

Instead, when I try to run crossplane beta render and specify the following manifest as extra resource:

apiVersion: platform.eng.it/v1alpha1
kind: Project
metadata:
  name: example-project
  namespace: default

I get a different result and the annotation is loaded:

apiVersion: repo.github.upbound.io/v1alpha1
kind: Repository
metadata:
  
  labels:
    test2: example-project

How can we reproduce it?

What environment did it happen in?

Function version: 0.10.1

@irizzant irizzant added the bug Something isn't working label Oct 16, 2024
@Peefy Peefy added good first issue Good for newcomers help wanted Extra attention is needed labels Oct 16, 2024
@Peefy Peefy reopened this Oct 17, 2024
@Peefy
Copy link
Collaborator

Peefy commented Oct 17, 2024

This is a strange question, and I'm not sure if the upstream community has encountered it before. I have confirmed that the logic in function-kcl is correct and the unit testing is passed. Perhaps it's a problem with the crossplane runtime. What is your crossplane version?

@irizzant
Copy link
Author

I'm running Crossplane 1.17.1 in kubernetes 1.30

@Peefy
Copy link
Collaborator

Peefy commented Oct 17, 2024

I see. Is it because the function-kcl version in the cluster is too low and does not support extraResources yet, which may result in different results from cross plane rendering?

@irizzant
Copy link
Author

irizzant commented Oct 17, 2024

You were probably right about the wrong function-kcl version, nice catch!

I made sure that version 0.10.1 is installed, and this is the current output showing the right version is now applied:

NAME                                          INSTALLED   HEALTHY   PACKAGE                                                              AGE
crossplane-contrib-function-auto-ready        True        True      xpkg.upbound.io/crossplane-contrib/function-auto-ready:v0.2.1        27m
crossplane-contrib-function-kcl               True        True      xpkg.upbound.io/crossplane-contrib/function-kcl:v0.10.1              27m

Now when I use this extra resources:

    apiVersion = "meta.krm.kcl.dev/v1alpha1"
    kind = "ExtraResources"
    requirements = {
        test = {
            apiVersion = "platform.eng.it/v1alpha1"
            kind = "Project"
            matchName = "example-project"
        }
    }
}]

I get a different error:

ReconcileError: cannot compose resources: cannot run Composition pipeline step "kcl": fetching resources for test: cannot get extra resource by name: an empty namespace may not be set when a resource name is provided

@irizzant
Copy link
Author

@Peefy I've also added this simple reproducer for the issue:
https://github.com/irizzante/function-kcl-issue-172

You can launch setup.sh and then:
kubectl describe testxr

@Peefy
Copy link
Collaborator

Peefy commented Oct 17, 2024

@irizzant I see. Thank you! I will check it later.
The extra resource feature was originally implemented in #162 by @fraenkel .

@fraenkel Sorry to bother you, I want to ask have you encountered similar issues?

@Peefy
Copy link
Collaborator

Peefy commented Oct 17, 2024

I've found the related issue: crossplane-contrib/function-extra-resources#27 @irizzant

@irizzant
Copy link
Author

Hi @Peefy
ok so function-extra-resources accepts only cluster-scoped resources, but what is the relation with this issue?

Does function-kcl use function-extra-resources under the hood?

Since the matchLabels field allows for namespaced resources, shouldn't it be fixed?

@Peefy
Copy link
Collaborator

Peefy commented Oct 17, 2024

https://github.com/crossplane/crossplane/blob/main/internal/controller/apiextensions/composite/extra_resources.go#L122

Errors seem emerge from the crossplane rather than in the function-kcl 🤔

@irizzant
Copy link
Author

Are you suggesting that in:
https://github.com/crossplane/crossplane/blob/e3b2f5685079f141d42400edd10848adff127930/internal/controller/apiextensions/composite/extra_resources.go#L122

there should be something like this?

nn := types.NamespacedName{Name: rs.GetMatchName(), Namespace: "your-namespace"}

@Peefy
Copy link
Collaborator

Peefy commented Oct 18, 2024

Yes. Or we can try to input a cluster scoped resource.

@irizzant
Copy link
Author

It doesn't look correct to me, given that you can specify a matchName one should be able to either specify a namespace or not to specify it at all

@Wompipomp
Copy link

I asked in Crossplane Slack. With extra resources you can only fetch composite (cluster scoped) resources. Here is the answer from one of the maintainer:

About the reasoning: it’s mainly because almost all crossplane resources are cluster-scoped, and you usually compose composite resources, except claims which are just a user-facing namespaced version of the composite resource though, and on top of that, it’s not easy to tell apart a claim resource from any other namespaced resource, so we’d have to allow accessi all namespaced resources, which would mean “secrets” too, which we preferred avoiding for obvious reasons.

You can fetch these composite resources either via name or labels. If you would like to fetch a composite resource via the claim name you could use matchlabels with crossplane.io/claim-name: claimname

But this should be maybe clarified in the docs that extra resources is specific to composite (cluster scoped) resources and does not work with claims 👍

@irizzant
Copy link
Author

Ok thank you for the clarifications

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
3 participants