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

feat: Using secret to attach identity to pod for observability publisher #607

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

tiopramayudi
Copy link
Contributor

Description

Since we are trying to move away from cloud-specific feature, hence as first step in observability publisher is to use secret for propagate identity instead of using existing workload identity way

Modifications

Add GOOGLE_APPLICATION_CREDENTIALS for propagate google identity to pod

Tests

Checklist

  • Added PR label
  • Added unit test, integration, and/or e2e tests
  • Tested locally
  • Updated documentation
  • Update Swagger spec if the PR introduce API changes
  • Regenerated Golang and Python client if the PR introduces API changes

Release Notes


@tiopramayudi tiopramayudi self-assigned this Sep 19, 2024
@tiopramayudi tiopramayudi added the enhancement New feature or request label Sep 19, 2024
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.75%. Comparing base (7caf48a) to head (b0040f3).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #607      +/-   ##
==========================================
+ Coverage   60.70%   60.75%   +0.04%     
==========================================
  Files         275      276       +1     
  Lines       22171    22205      +34     
==========================================
+ Hits        13460    13491      +31     
- Misses       7847     7849       +2     
- Partials      864      865       +1     
Flag Coverage Δ
api-test 58.73% <ø> (+0.05%) ⬆️
sdk-test-3.10 75.51% <ø> (ø)
sdk-test-3.8 75.49% <ø> (ø)
sdk-test-3.9 75.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the changes @tiopramayudi ! I just left a couple of tiny comments but the main one's about removing the support for workload identity completely 🤔 (If it's not the scope of this MR or if it's something that isn't planned it's okay too 😅)

api/pkg/observability/deployment/deployment_test.go Outdated Show resolved Hide resolved
api/config/observability.go Show resolved Hide resolved
api/pkg/observability/deployment/identity.go Show resolved Hide resolved
api/pkg/observability/deployment/identity.go Outdated Show resolved Hide resolved
@tiopramayudi tiopramayudi merged commit 596b887 into main Oct 2, 2024
35 checks passed
@tiopramayudi tiopramayudi deleted the sa-from-secret branch October 2, 2024 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants