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.

Sign up for GitHub

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

Jump to bottom

Set custom path for mounted ServiceAccount token #1077

Merged
merged 8 commits into from
Jun 1, 2023
Merged

Set custom path for mounted ServiceAccount token #1077

merged 8 commits into from
Jun 1, 2023

Conversation

huseyinbabal
Copy link
Contributor

@huseyinbabal huseyinbabal commented May 30, 2023 •
edited
Loading

Description

Changes proposed in this pull request:

  • Default token path for botkube deployment is changed. Now we use uuid in the token path

Testing

  • gh pr checkout 1077 --repo=https://github.com/kubeshop/botkube
  • Create an instance in Botkube Cloud
  • Run following
helm upgrade --install botkube \
  --version v1.0.1 \
  --namespace botkube \
  --create-namespace \
  --wait \
  --set config.provider.endpoint=https://api.botkube.io/graphql \
  --set config.provider.identifier=... \
  --set config.provider.apiKey=... \
  ./helm/botkube

Related issue(s)

Relates #1021

@huseyinbabal huseyinbabal marked this pull request as ready for review May 30, 2023 08:53
@huseyinbabal huseyinbabal requested a review from a team May 30, 2023 08:53
@huseyinbabal huseyinbabal requested a review from PrasadG193 as a code owner May 30, 2023 08:53
@huseyinbabal huseyinbabal requested a review from pkosiec May 30, 2023 08:53
@pkosiec pkosiec self-assigned this May 30, 2023
pkosiec
pkosiec reviewed May 30, 2023
View reviewed changes
Copy link
Collaborator

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

I like the path randomizaton approach 👍 However, it won't work correctly for now as we still have the automounted token from SA. Please apply the changes and let me know about it 🙂

helm/botkube/templates/deployment.yaml Show resolved Hide resolved
pkosiec
pkosiec approved these changes Jun 1, 2023
View reviewed changes
Copy link
Collaborator

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

pkg/config/config.go Outdated
Log Logger `yaml:"log"`
InformersResyncPeriod time.Duration `yaml:"informersResyncPeriod"`
Kubeconfig string `yaml:"kubeconfig"`
SaCredentialsPathPrefix string `yaml:"saCredentialsPathPrefix"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider different casing:

Suggested change
SaCredentialsPathPrefix string `yaml:"saCredentialsPathPrefix"`
SACredentialsPathPrefix string `yaml:"saCredentialsPathPrefix"`

(you can ignore this comment if you don't like it)

internal/kubex/rest.go Outdated
"k8s.io/klog/v2"
)

// BuildConfigFromFlags Kubeconfig config initialization
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also copied, right? Could you also add a link to the source?

pkosiec
pkosiec approved these changes Jun 1, 2023
View reviewed changes
Copy link
Collaborator

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@huseyinbabal huseyinbabal merged commit faab20a into kubeshop:main Jun 1, 2023
@huseyinbabal huseyinbabal deleted the custom-sa-token-path branch June 1, 2023 09:14
@pkosiec pkosiec changed the title custom sa token path Set custom path for mounted ServiceAccount token Jun 15, 2023
@pkosiec pkosiec added the enhancement New feature or request label Jun 15, 2023
@trevorlauder trevorlauder mentioned this pull request Jul 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkosiec pkosiec pkosiec approved these changes

@PrasadG193 PrasadG193 Awaiting requested review from PrasadG193 PrasadG193 is a code owner

Assignees

@pkosiec pkosiec

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@huseyinbabal @pkosiec