Skip to content

feat: POST /conversations to companion backend#3691

Merged
mrCherry97 merged 23 commits intokyma-project:mainfrom
chriskari:post-conversations
Mar 4, 2025
Merged

feat: POST /conversations to companion backend#3691
mrCherry97 merged 23 commits intokyma-project:mainfrom
chriskari:post-conversations

Conversation

@chriskari
Copy link
Contributor

@chriskari chriskari commented Feb 12, 2025

Description

Changes proposed in this pull request:

  • track currently opened resource through columnLayoutState (added startColumn)
  • adjusted format of API request
  • API request is proxied through our backend
  • TokenManager (refetching etc.) added to backend
  • suggestions will refresh when changing resource
  • use correct Authorization headers (token vs. client-certificate and client-key)

Related issue(s)
#3602

Definition of done

  • The PR's title starts with one of the following prefixes:
    • feat: A new feature
    • fix: A bug fix
    • docs: Documentation only changes
    • refactor: A code change that neither fixes a bug nor adds a feature
    • test: Adding tests
    • revert: Revert commit
    • chore: Maintainance changes to the build process or auxiliary tools, libraries, workflows, etc.
  • Related issues are linked. To link internal trackers, use the issue IDs like backlog#4567
  • Explain clearly why you created the PR and what changes it introduces
  • All necessary steps are delivered, for example, tests, documentation, merging

@kyma-bot kyma-bot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 12, 2025
@chriskari chriskari linked an issue Feb 12, 2025 that may be closed by this pull request
6 tasks
@kyma-bot kyma-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 12, 2025
@kyma-bot kyma-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 13, 2025
@kyma-bot kyma-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 19, 2025
@dbadura
Copy link
Contributor

dbadura commented Feb 21, 2025

I started it, use local k3d and got errors when opening the companion chat.

[backend]   const clusterToken = req.headers['x-k8s-authorization'].replace(/^Bearer\s+/i, '');
[backend]                                                           ^
[backend] 
[backend] TypeError: Cannot read properties of undefined (reading 'replace')
[backend]     at handleAIChatRequest (/home/damian/Workspace/kyma/busola/backend/companion/companionRouter.js:15:59)

Probably, the backend couldn't use credentials to backend, but it shouldn't break like that.

Small update:
After opening the chat, the whole communication with backend is broken:
image

try {
// Split the token and get the payload
const payload = JSON.parse(
Buffer.from(token.split('.')[1], 'base64').toString(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Use const like PAYLOAD_INDEX instead of magic number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

req.body.toString(),
);
const clusterUrl = req.headers['x-cluster-url'];
const clusterToken = req.headers['x-k8s-authorization'].replace(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is regex required to use?

Copy link
Contributor Author

@chriskari chriskari Feb 28, 2025

Choose a reason for hiding this comment

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

yes this is the cleanest approach

@chriskari
Copy link
Contributor Author

chriskari commented Feb 23, 2025

Hey Damian, thanks for the review.
The reason why your API requests to the companion backend are failing is that when connecting to your local k3d cluster, you are authenticating using x-client-key-data instead of a x-k8s-authorization token. Currently the companion backend however expects us to send a token to authenticate to the Kubernetes cluster. They do not handle the case where users authenticate without an authentication token but with key-data. Because of that I have not yet implemented this either.
Apart from that fact, because your cluster is hosted locally, the companion backend can not access any cluster information as the x-cluster-url points to localhost.

Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

It will be nice to have some Loader/Spinner when Busola is loading suggestions, because I was confused in first couple seconds that it's not working.

is it intended, that if I go on PersistentVolumes -> Open AI -> I have suggestions for PV -> Go to StorageClasses or somewhere else -> suggestions are not refreshed, shouldn't suggestions get refreshed on the screen change?

Copy link
Contributor

@mrCherry97 mrCherry97 left a comment

Choose a reason for hiding this comment

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

Could you also change js files to ts? like TokenManager, router and gettoken?

Copy link
Contributor

@akucharska akucharska left a comment

Choose a reason for hiding this comment

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

  1. Suggestions are only refreshed when you close and open joule again. It should refresh if you go to different page. If it is not possible to start/change resource then maybe close joule?

  2. It would be good to have some indicator if suggestions are loading, and some message if there is no suggestions to show (on modules).

  3. On the button opening Joule we popup saying what this button is about. It would be good to have the same on buttons in Joule window
    Screenshot 2025-02-26 at 11 26 23

@dbadura
Copy link
Contributor

dbadura commented Feb 27, 2025

Hey Damian, thanks for the review. The reason why your API requests to the companion backend are failing is that when connecting to your local k3d cluster, you are authenticating using x-client-key-data instead of a x-k8s-authorization token. Currently the companion backend however expects us to send a token to authenticate to the Kubernetes cluster. They do not handle the case where users authenticate without an authentication token but with key-data. Because of that I have not yet implemented this either. Apart from that fact, because your cluster is hosted locally, the companion backend can not access any cluster information as the x-cluster-url points to localhost.

I understand that the companion can't access the local cluster, but the request from the bottom of the screenshot to normal backend fails after opening a chat.
image

@chriskari chriskari requested a review from a team as a code owner March 3, 2025 14:54
@kyma-bot kyma-bot added the lgtm Looks good to me! label Mar 4, 2025
@mrCherry97 mrCherry97 enabled auto-merge (squash) March 4, 2025 10:28
@mrCherry97 mrCherry97 dismissed akucharska’s stale review March 4, 2025 10:31

Requested changes are adjusted.

@mrCherry97 mrCherry97 merged commit 7733af6 into kyma-project:main Mar 4, 2025
36 checks passed
@chriskari chriskari deleted the post-conversations branch March 10, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the CLA. lgtm Looks good to me! size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate and adjust POST /conversations endpoint

7 participants