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(config): use projected volume for SA token #3563

Merged
merged 1 commit into from
Feb 15, 2023
Merged

Conversation

czeslavo
Copy link
Contributor

@czeslavo czeslavo commented Feb 15, 2023

What this PR does / why we need it:

Ports the changes from helm chart: Kong/charts#722. A projected volume is used instead of a static secret.

That effectively makes our manifests not compatible with Kubernetes < 1.21 versions. According to our support matrix, we already do not support such.

E2E tests run: https://github.com/Kong/kubernetes-ingress-controller/actions/runs/4187269214

Which issue this PR fixes:

Closes #3475.

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@czeslavo czeslavo self-assigned this Feb 15, 2023
@czeslavo czeslavo added area/feature New feature or request area/maintenance Cleanup, refactoring, and other maintenance improvements that don't change functionality. labels Feb 15, 2023
@czeslavo czeslavo added this to the KIC v2.9.0 milestone Feb 15, 2023
@czeslavo czeslavo marked this pull request as ready for review February 15, 2023 19:07
@czeslavo czeslavo requested a review from a team as a code owner February 15, 2023 19:07
@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Base: 72.1% // Head: 72.2% // Increases project coverage by +0.0% 🎉

Coverage data is based on head (78e0490) compared to base (56a11b9).
Patch has no changes to coverable lines.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #3563   +/-   ##
=====================================
  Coverage   72.1%   72.2%           
=====================================
  Files        127     127           
  Lines      14771   14771           
=====================================
+ Hits       10660   10668    +8     
+ Misses      3434    3427    -7     
+ Partials     677     676    -1     
Impacted Files Coverage Δ
...nternal/controllers/gateway/tcproute_controller.go 70.0% <0.0%> (-2.0%) ⬇️
...ternal/controllers/gateway/httproute_controller.go 69.7% <0.0%> (-1.9%) ⬇️
internal/dataplane/kongstate/service.go 66.0% <0.0%> (-1.3%) ⬇️
internal/dataplane/parser/parser.go 90.7% <0.0%> (+0.6%) ⬆️
internal/dataplane/kongstate/util.go 92.5% <0.0%> (+1.5%) ⬆️
...nternal/controllers/gateway/udproute_controller.go 73.7% <0.0%> (+2.1%) ⬆️
internal/dataplane/kongstate/kongstate.go 73.9% <0.0%> (+4.5%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

🏅

@pmalek
Copy link
Member

pmalek commented Feb 15, 2023

Since we're recently making more and more changes that require an e2e pass and we have no way of verifying that outside of the bounds of PRs we should add an e2e run label similarly to what we have for nightly: https://github.com/Kong/kubernetes-ingress-controller/blob/ec17ebcaa39fdfd8a01d960eaaadcc2546e4da9a/.github/workflows/test_nightly.yaml

@czeslavo czeslavo merged commit 697d608 into main Feb 15, 2023
@czeslavo czeslavo deleted the projected-sa-volume branch February 15, 2023 19:52
Copy link
Contributor

@seh seh left a comment

Choose a reason for hiding this comment

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

Note that the Secret named "kong-serviceaccount-token" still winds up in the set of generated manifests:

apiVersion: v1
kind: Secret
metadata:
annotations:
kubernetes.io/service-account.name: kong-serviceaccount
name: kong-serviceaccount-token
namespace: kong
type: kubernetes.io/service-account-token

@rainest's #3475 didn't mention removing this Secret, but I took it to be implied.

@czeslavo
Copy link
Contributor Author

Note that the Secret named "kong-serviceaccount-token" still winds up in the set of generated manifests:

apiVersion: v1
kind: Secret
metadata:
annotations:
kubernetes.io/service-account.name: kong-serviceaccount
name: kong-serviceaccount-token
namespace: kong
type: kubernetes.io/service-account-token

@rainest's #3475 didn't mention removing this Secret, but I took it to be implied.

@seh Thanks for pointing this out, I overlooked it's not needed anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/feature New feature or request area/maintenance Cleanup, refactoring, and other maintenance improvements that don't change functionality. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port projected volume SA token from chart
3 participants