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

Improve cross-namespace check when referencing Kamelets in Pipes #4961

Closed
christophd opened this issue Dec 4, 2023 · 6 comments · Fixed by #4965
Closed

Improve cross-namespace check when referencing Kamelets in Pipes #4961

christophd opened this issue Dec 4, 2023 · 6 comments · Fixed by #4965
Labels
kind/bug Something isn't working status/waiting-for-feedback Needs some feedback

Comments

@christophd
Copy link
Contributor

Requirement

When referencing Kamelets in a Pipe or KameletBinding the operator makes sure to not use Kamelets from another namespace. The check raises errors such as cross-namespace references are not allowed.

We should not raise the error when the user explicitly references a Kamelet from the operator namespace as these Kamelets represent cluster wide defaults that all users may use.

Problem

The check also raises the error when the user references Kamelets from the operator namespace which is allowed of course. The simple fact that the user has given the operator namespace in the Kamelet reference causes the check to fail because the namespace does not match the current user namespace (cross-namespace references are not allowed).

Proposal

Improve the check logic to account for Kamelet references that explicitly use the operator namespace and do not raise the error in this scenario.

Open questions

No response

@christophd christophd added the kind/feature New feature or request label Dec 4, 2023
@christophd
Copy link
Contributor Author

@squakez
Copy link
Contributor

squakez commented Dec 5, 2023

In theory it should work when installing a global operator: #1675

I'm marking as a bug in order to double check that the above is still working. If not, we need to fix it.

@squakez squakez added kind/bug Something isn't working and removed kind/feature New feature or request labels Dec 5, 2023
@squakez
Copy link
Contributor

squakez commented Dec 5, 2023

I've just double checked this and it works as expected. Installing the operator (in global mode) in a namespace (ie, default) and running the Integration in a different namespace (ie, test), it manages to run the integration correctly. Also, it reports the condition accordingly:

    - firstTruthyTime: "2023-12-05T09:29:33Z"
      lastTransitionTime: "2023-12-05T09:29:33Z"
      lastUpdateTime: "2023-12-05T09:29:33Z"
      message: kamelets [log-sink,timer-source] found in (Kubernetes[namespace=test],
        Kubernetes[namespace=default], Empty[]) repositories
      reason: KameletsAvailable
      status: "True"
      type: KameletsAvailable

Can you please provide the exact steps in which the issue reported here is failing?

@squakez squakez added the status/waiting-for-feedback Needs some feedback label Dec 5, 2023
@christophd
Copy link
Contributor Author

You should be able to reproduce with this Pipe:

apiVersion: camel.apache.org/v1
kind: Pipe
metadata:
  name: timer-to-log
spec:
  source:
    ref:
      kind: Kamelet
      namespace: default
      apiVersion: camel.apache.org/v1
      name: timer-source
    properties:
      message: "Hello from Camel K!"
  sink:
    ref:
      kind: Kamelet
      namespace: default
      apiVersion: camel.apache.org/v1
      name: log-sink

It is because the reference to the Kamelets specifies the 'default' namespace (which is the global operator namespace). When you run that Pipe from test namespace it should fail because of the check.

@squakez
Copy link
Contributor

squakez commented Dec 5, 2023

Got it. Yeah, the problem is the declaration of the namespace. You need to use the Kamelets as if they were local to the namespace, the global operator will be able to look into its own namespace and make it work. I think it was the way this feature was designed, however it's not document anywhere apparently.

The above would work using:

apiVersion: camel.apache.org/v1
kind: Pipe
metadata:
  name: timer-to-log
spec:
  source:
    ref:
      kind: Kamelet
      namespace: test
      apiVersion: camel.apache.org/v1
      name: timer-source
    properties:
      message: "Hello from Camel K!"
  sink:
    ref:
      kind: Kamelet
      namespace: test
      apiVersion: camel.apache.org/v1
      name: log-sink

@christophd
Copy link
Contributor Author

I really wonder though why there is a namespace option on the Kamelet ref at all. Both valid values (the user namespace and the operator namespace) are already covered by the operator logic when the namespace is not given, because the operator searches for the Kamelet in the user namespace first and then searches its own namespace as a fallback if I am correct

christophd added a commit to christophd/camel-k that referenced this issue Dec 5, 2023
- Allow explicit references to default Kamelets in operator namespace
- Keep check on cross-namespace reference for other resources (e.g. Knative Broker)
- Keep raising cross-namespace check errors for Kamelet references to other user namespaces
christophd added a commit to christophd/camel-k that referenced this issue Dec 5, 2023
- Allow explicit references to default Kamelets in operator namespace
- Keep check on cross-namespace reference for other resources (e.g. Knative Broker)
- Keep raising cross-namespace check errors for Kamelet references to other user namespaces
@squakez squakez linked a pull request Dec 5, 2023 that will close this issue
squakez pushed a commit that referenced this issue Dec 5, 2023
- Allow explicit references to default Kamelets in operator namespace
- Keep check on cross-namespace reference for other resources (e.g. Knative Broker)
- Keep raising cross-namespace check errors for Kamelet references to other user namespaces
christophd added a commit to christophd/camel-k that referenced this issue Dec 5, 2023
- Allow explicit references to default Kamelets in operator namespace
- Keep check on cross-namespace reference for other resources (e.g. Knative Broker)
- Keep raising cross-namespace check errors for Kamelet references to other user namespaces
squakez pushed a commit that referenced this issue Dec 7, 2023
- Allow explicit references to default Kamelets in operator namespace
- Keep check on cross-namespace reference for other resources (e.g. Knative Broker)
- Keep raising cross-namespace check errors for Kamelet references to other user namespaces
squakez pushed a commit to jboss-fuse/camel-k that referenced this issue Dec 7, 2023
- Allow explicit references to default Kamelets in operator namespace
- Keep check on cross-namespace reference for other resources (e.g. Knative Broker)
- Keep raising cross-namespace check errors for Kamelet references to other user namespaces

(cherry picked from commit apache/camel-k@0dafa9b9c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working status/waiting-for-feedback Needs some feedback
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants