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

Add jitter to Default token expiration #260

Merged
merged 5 commits into from
Mar 4, 2025
Merged

Add jitter to Default token expiration #260

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
611c1f7
Adding jitter based on percent to default token expiration to avoid t…
notSoWiseOldMan Feb 28, 2025
04fae22
Adding mockery configs and math/rand mocks
notSoWiseOldMan Mar 1, 2025
3eb38ec
Adding Jitter and updating tests cases for token expiration jitter
notSoWiseOldMan Mar 1, 2025
e55d30e
Remove jitter from ISRAv1 code path
notSoWiseOldMan Mar 3, 2025
ed07698
Fixing test data with randomized token expiration
notSoWiseOldMan Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ permissions and limitations under the License.
package pkg

const (
// Default token expiration in seconds if none is defined,
// which is 24hrs as that is max for EKS
DefaultTokenExpiration = int64(86400)
// 24hrs as that is max for EKS
MaxTokenExpiration = int64(86400)
// Default token expiration in seconds if none is defined, 22hrs
DefaultTokenExpiration = int64(79200)
Copy link
Member

Choose a reason for hiding this comment

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

The DefaultTokenExpiration is used by both IRSAv1 and IRSAv2 (Pod Identity):

  1. In IRSAv1, it has one informer cache
  2. In IRSAv2, we are only using from GetCommonConfigurations, which is being added from onboarding to IRSAv2 Add support for container credentials method #189

I believe we can add the randomization inside the GetCommonConfigurations function and not change the DefaultTokenExpiration for IRSAv1 use

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I have updated GetCommonConfigurations the way you are describing in the most recent commit now. Let me know if I missed somethign

// 10mins is min for kube-apiserver
MinTokenExpiration = int64(600)

Expand Down
29 changes: 29 additions & 0 deletions pkg/handler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"math/rand"
"net/http"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -417,6 +418,14 @@ func (m *Modifier) buildPodPatchConfig(pod *corev1.Pod) *podPatchConfig {
regionalSTS, tokenExpiration := m.Cache.GetCommonConfigurations(pod.Spec.ServiceAccountName, pod.Namespace)
tokenExpiration, containersToSkip := m.parsePodAnnotations(pod, tokenExpiration)

if tokenExpiration == pkg.DefaultTokenExpiration {
klog.V(4).Infof("Adding jitter to default token expiration")
var err error
tokenExpiration, err = addJitter(tokenExpiration, 5, pkg.MinTokenExpiration, pkg.MaxTokenExpiration)
if err != nil {
klog.Errorf("Error adding jitter to default token expiration: %v", err)
}
}
webhookPodCount.WithLabelValues("container_credentials").Inc()

return &podPatchConfig{
Expand Down Expand Up @@ -479,6 +488,26 @@ func (m *Modifier) buildPodPatchConfig(pod *corev1.Pod) *podPatchConfig {
return nil
}

func addJitter(val int64, jitterPercent int64, min int64, max int64) (int64, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be just simplified to a randomization between min and max, we don't need the val. And jitterPercentage and (min, max) is kinda similar, just to add randomization

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rewrote the function and renamed to addJitterToDefaultToken now that it is more specific

if max < min {
return val, error(fmt.Errorf("max value %d is less than min value %d, cannot add jitter", max, min))
}

jitterFactor := float64(jitterPercent) / 100.0
jitterMin := int64(float64(val) - (float64(val) * jitterFactor))
if jitterMin < min {
jitterMin = min
}
jitterMax := int64(float64(val) + (float64(val) * jitterFactor))
if jitterMax > max {
jitterMax = max
}

valWithJitter := rand.Int63n(jitterMax - jitterMin + 1) + jitterMin

return valWithJitter, nil
}

// MutatePod takes a AdmissionReview, mutates the pod, and returns an AdmissionResponse
func (m *Modifier) MutatePod(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
badRequest := &v1beta1.AdmissionResponse{
Expand Down
18 changes: 18 additions & 0 deletions pkg/handler/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,24 @@ func serializeAdmissionReview(t *testing.T, want *v1beta1.AdmissionReview) []byt
return wantedBytes
}

func TestAddJitterMinMax(t *testing.T) {
var (
min int64
max int64
)
min, max = 8, 11
for i := 0; i < 10; i++ {
jitter, err := addJitter(10, 1000, min, max)
assert.True(t, jitter >= min && jitter <= max)
assert.True(t, err == nil)
}
}

func TestAddJitterMinGTMax(t *testing.T) {
_, err := addJitter(10, 1000, 11, 8)
assert.True(t, err != nil)
}

func TestModifierHandler(t *testing.T) {
testServiceAccount := &corev1.ServiceAccount{}
testServiceAccount.Name = "default"
Expand Down
Loading