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

Enhancement: Implement RBAC policy and Service Account for relay service #1544

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

Utkar5hM
Copy link
Contributor

Purpose of PR?:

Fixes #1525

Does this PR introduce a breaking change?

If the changes in this PR are manually verified, list down the scenarios covered::
Verified and found that the only place where the application seems to be using the kubernetes API is to fetch a list of all the pods from all the namespaces while rest of the process takes place via gRPC and other means.

I initially tried configuring Role/RoleBinding for implementing RBAC policy considering both kubearmor and kubearmor-relay will be deployed in the same namespace kubearmor hoping the relay server would still work fetching only the pods from the kubearmor namespace but it fails (doesn't display the error in std error output). To further debug, I tried fetching pods list with kubectl as the serviceaccount and it failed as well making sure that the kubernetes API doesn't allow fetching the entire list with Role and would require code change.

kubectl get pods -A --as=system:serviceaccount:kubearmor:kubearmor-relay
Error from server (Forbidden): pods is forbidden: User "system:serviceaccount:kubearmor:kubearmor-relay" cannot list resource "pods" in API group "" at the cluster scope

Due to which I went ahead and configured ClusterRole, which succeeded with the same command and kubearmor-relay was able to detect the pods.

image

Additional information for reviewer? :

Checklist:

  • Bug fix. Fixes #
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Commit has unit tests
  • Commit has integration tests

@Utkar5hM
Copy link
Contributor Author

@rksharma95 , Are any further changes required? If not then should I proceed to make a PR to relay-server's repository to update relay-deployment.yaml to use the same service account and policy?

@rksharma95
Copy link
Collaborator

hey @Utkar5hM thanks for the PR, it looks good. can you also handle the deployment change with operator too? basically you need to add the new service account and RBAC roles to the resource watcher.

func (clusterWatcher *ClusterWatcher) WatchRequiredResources() {

@Utkar5hM
Copy link
Contributor Author

@rksharma95, I've Updated the Operator and made a PR in relay-server's repository to update the changes in the yaml.

Copy link
Member

@PrimalPimmy PrimalPimmy left a comment

Choose a reason for hiding this comment

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

LGTM

@Utkar5hM
Copy link
Contributor Author

Utkar5hM commented Dec 27, 2023

Is there an existing issue in the pipeline or with the controller? I tried looking at the logs for why the ci-test-controllers is failing but while doing so, I noticed that the action seems to be failing for few other PR's as well.

So I tried running it on the main branch(updated) by adding a empty file in my fork and it seems to fail at the same stage.

@rksharma95
Copy link
Collaborator

@Utkar5hM can you rebase the branch to main.

@rksharma95
Copy link
Collaborator

@Utkar5hM can you please rebase again 😁
we merged two PRs after that and hoping to get this issue resolved.

@rksharma95
Copy link
Collaborator

@Utkar5hM please squash the commits then it's good to merge.

Signed-off-by: Utkarsh Mahajan <utkarshrm568@gmail.com>
Copy link
Collaborator

@rksharma95 rksharma95 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@rksharma95 rksharma95 merged commit c2c1b12 into kubearmor:main Jan 10, 2024
16 checks passed
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.

Implement RBAC Policy and Service Account for KubeArmor-Relay
3 participants