Skip to content

Conversation

Muchembi
Copy link
Contributor

@Muchembi Muchembi commented Apr 9, 2025

  • Breaking change? (if so, please describe the impact and migration path for existing application instances)

What changes did you make? (Give an overview)
resolves #1012

Is there anything you'd like reviewers to focus on?
The front end especially.

How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)

  • No need to
  • Manually - Built an image and used it in a k8s helm deployment. I was able to authenticate to my GMSK and interact with the cluster as normal.
  • Unit checks
  • Integration checks
  • Covered by existing automation

Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. ENVIRONMENT VARIABLES)
  • My changes generate no new warnings (e.g. Sonar is happy)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged

Check out Contributing and Code of Conduct

A picture of a cute animal (not mandatory but encouraged)
178BA733-EBF5-4E47-9D43-29BDFA1943FB

@Muchembi Muchembi requested review from a team as code owners April 9, 2025 11:54
@kapybro kapybro bot added status/triage Issues pending maintainers triage status/triage/manual Manual triage in progress status/triage/completed Automatic triage completed and removed status/triage Issues pending maintainers triage labels Apr 9, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi Muchembi! 👋

Welcome, and thank you for opening your first PR in the repo!

Please wait for triaging by our maintainers.

Please take a look at our contributing guide.

@Muchembi Muchembi mentioned this pull request Apr 9, 2025
2 tasks
@Muchembi Muchembi changed the title Support for Google Managed Service for Apache Kafka Support for GCP IAM Auth for Google Managed Service for Apache Kafka Apr 9, 2025
@Haarolean Haarolean self-requested a review April 9, 2025 12:23
@Haarolean Haarolean added type/enhancement En enhancement/improvement to an already existing feature scope/backend Related to backend changes area/auth App authentication related issues and removed status/triage/manual Manual triage in progress labels Apr 9, 2025
@Haarolean Haarolean added this to the 1.3 milestone Apr 9, 2025
@Haarolean Haarolean changed the title Support for GCP IAM Auth for Google Managed Service for Apache Kafka BE: Auth: Support GCP IAM Auth Apr 9, 2025
@Haarolean Haarolean moved this from Todo to In Review in Release 1.3 Apr 9, 2025
@germanosin germanosin self-requested a review April 9, 2025 15:53
Copy link
Member

@Haarolean Haarolean left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. Can we also get a test like AzureEntraLoginCallbackHandlerTest but for GCP? Feel free to mock it heavily.

@github-project-automation github-project-automation bot moved this from In Review to Changes requested in Release 1.3 Apr 16, 2025
@Haarolean
Copy link
Member

@Muchembi also, could you please update our docs to reflect new changes?
https://github.com/kafbat/ui-docs

@Muchembi
Copy link
Contributor Author

@Muchembi also, could you please update our docs to reflect new changes? https://github.com/kafbat/ui-docs

Sorry @Haarolean. I missed this one. Let me add that to the PR over the course of the day.

@Muchembi
Copy link
Contributor Author

@Haarolean Dropped a PR the docs.

@Muchembi
Copy link
Contributor Author

Thanks for your PR. Can we also get a test like AzureEntraLoginCallbackHandlerTest but for GCP? Feel free to mock it heavily.

@Haarolean Looking at AzureEntraLoginCallbackHandlerTest. I am wondering if there is need to implement a similar test here while the test is already implemented in the packaged lib here:
https://github.com/googleapis/managedkafka/blob/main/kafka-java-auth/src/test/java/com/google/cloud/hosted/kafka/auth/GcpLoginCallbackHandlerTest.java

@Haarolean
Copy link
Member

Thanks for your PR. Can we also get a test like AzureEntraLoginCallbackHandlerTest but for GCP? Feel free to mock it heavily.

@Haarolean Looking at AzureEntraLoginCallbackHandlerTest. I am wondering if there is need to implement a similar test here while the test is already implemented in the packaged lib here: https://github.com/googleapis/managedkafka/blob/main/kafka-java-auth/src/test/java/com/google/cloud/hosted/kafka/auth/GcpLoginCallbackHandlerTest.java

I see, let's skip this then, thanks!

@Haarolean Haarolean self-requested a review May 1, 2025 12:30
@github-project-automation github-project-automation bot moved this from Changes requested to PR Approved in Release 1.3 May 1, 2025
@Haarolean Haarolean enabled auto-merge (squash) May 1, 2025 12:31
@Haarolean Haarolean merged commit b768df6 into kafbat:main May 1, 2025
16 checks passed
@github-project-automation github-project-automation bot moved this from PR Approved to Done in Release 1.3 May 1, 2025
@mike-pt
Copy link
Contributor

mike-pt commented May 13, 2025

I know this is already merged but since its recent please note that I see this in UI (using dynamic config at start)

image

It seems like in the frontend it might not have been added in this check, let me know if I should open a GHI, I didn't since this seems to be WIP

@Haarolean
Copy link
Member

@mike-pt I'll reopen #1012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/auth App authentication related issues scope/backend Related to backend changes status/triage/completed Automatic triage completed type/enhancement En enhancement/improvement to an already existing feature
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Auth: Support for GCP IAM
4 participants