Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

support loading client secrets from Kubernetes Secret Resource #26

Merged
merged 3 commits into from
Feb 24, 2024

Conversation

zhaohuabing
Copy link
Contributor

@zhaohuabing zhaohuabing commented Feb 20, 2024

This PR loads client secrets from Kubernetes Secrets when reading config.

A follow-up PR will be added to watch Kubernetes Secrets and update the config.

Fix #24

@zhaohuabing zhaohuabing marked this pull request as draft February 20, 2024 06:40
@zhaohuabing zhaohuabing force-pushed the k8s-secret branch 2 times, most recently from b03c5de to 4674174 Compare February 20, 2024 06:55
@zhaohuabing zhaohuabing force-pushed the k8s-secret branch 5 times, most recently from 71f6729 to e2dd0bb Compare February 21, 2024 07:25
Copy link

codecov bot commented Feb 21, 2024

Codecov Report

Attention: Patch coverage is 91.11111% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 93.05%. Comparing base (27eba7b) to head (3e45ec5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
- Coverage   93.12%   93.05%   -0.07%     
==========================================
  Files          19       20       +1     
  Lines        1266     1311      +45     
==========================================
+ Hits         1179     1220      +41     
- Misses         51       55       +4     
  Partials       36       36              
Files Coverage Δ
internal/secrets.go 91.11% <91.11%> (ø)

@zhaohuabing zhaohuabing marked this pull request as ready for review February 21, 2024 07:38
@zhaohuabing zhaohuabing force-pushed the k8s-secret branch 2 times, most recently from dd944e9 to c4a373d Compare February 21, 2024 12:28
@zhaohuabing zhaohuabing force-pushed the k8s-secret branch 2 times, most recently from ad614bc to dea5e25 Compare February 21, 2024 12:45
Copy link
Member

@nacx nacx left a comment

Choose a reason for hiding this comment

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

Added some comments. Thanks!

internal/authz/oidc.go Outdated Show resolved Hide resolved
internal/authz/oidc.go Outdated Show resolved Hide resolved
internal/authz/oidc_kubernetes_test.go Outdated Show resolved Hide resolved
internal/authz/oidc_kubernetes_test.go Outdated Show resolved Hide resolved
@zhaohuabing zhaohuabing force-pushed the k8s-secret branch 2 times, most recently from 036d5f8 to 848534d Compare February 23, 2024 05:51
@zhaohuabing zhaohuabing marked this pull request as draft February 23, 2024 05:53
@zhaohuabing zhaohuabing force-pushed the k8s-secret branch 7 times, most recently from 5f0bd3b to ab4b0f4 Compare February 23, 2024 06:49
@zhaohuabing zhaohuabing marked this pull request as ready for review February 23, 2024 06:55
}

return cl, nil
}
Copy link
Member

Choose a reason for hiding this comment

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

We will be loading other secrets, and I think we could decouple the K8s logic from the Config file itself, for a cleaner design. For example, we could:

  • Create a SecretLoader in the internal package that receives the Config in the constructor and that implements the run.PreRunner interface. This will make its PreRun method be called after the configuration has been loaded.
  • In the PreRun method, you can load the secrets and mutate the configuration accordingly.
  • In the main.go, register the secretLoader in the run group immediately after the localConfig.

This way the k8s logic will be decoupled from the configuration, and if we need to reuse, say the k8s client in other services we'll already have the code prepared to share it more easily.

@nacx
Copy link
Member

nacx commented Feb 24, 2024

Added a small fix to the e2e tests, so that the kubeconfig is only loaded if there is any secret ref configured. It was trying to load it in the docker-based tests as well.
FYI: when e2e-tests fail, artifacts with all the logs are attached to the CI build, for further inspection.

@nacx nacx enabled auto-merge (squash) February 24, 2024 06:04
@nacx nacx merged commit c758258 into main Feb 24, 2024
10 of 11 checks passed
@nacx nacx deleted the k8s-secret branch February 24, 2024 06:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support loading certs from Kubernetes secrets
3 participants