From dc6632947dea9c2a334d8bc4fad9f95ba5f82ba3 Mon Sep 17 00:00:00 2001 From: Dhouib-Mohamed Date: Tue, 6 Aug 2024 15:52:26 +0100 Subject: [PATCH 1/4] feat(credentials): add Image Credentials as supported credentail method Signed-off-by: Dhouib-Mohamed --- internal/credentials/kubernetes/github/app.go | 2 +- .../credentials/kubernetes/github/app_test.go | 134 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/internal/credentials/kubernetes/github/app.go b/internal/credentials/kubernetes/github/app.go index 9db1d1fba..b203d644c 100644 --- a/internal/credentials/kubernetes/github/app.go +++ b/internal/credentials/kubernetes/github/app.go @@ -56,7 +56,7 @@ func (a *appCredentialHelper) getCredentials( _ string, secret *corev1.Secret, ) (*credentials.Credentials, error) { - if credType != credentials.TypeGit || secret == nil { + if (credType != credentials.TypeGit && credType != credentials.TypeImage) || secret == nil { // This helper can't handle this return nil, nil } diff --git a/internal/credentials/kubernetes/github/app_test.go b/internal/credentials/kubernetes/github/app_test.go index e17cc8f19..0aed1d299 100644 --- a/internal/credentials/kubernetes/github/app_test.go +++ b/internal/credentials/kubernetes/github/app_test.go @@ -48,8 +48,26 @@ func TestAppCredentialHelper(t *testing.T) { require.Nil(t, creds) }, }, + { + name: "cred type is git", + credType: credentials.TypeGit, + helper: &appCredentialHelper{}, + assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { + require.NoError(t, err) + require.Nil(t, creds) + }, + }, { name: "secret is nil", + credType: credentials.TypeImage, + helper: &appCredentialHelper{}, + assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { + require.NoError(t, err) + require.Nil(t, creds) + }, + }, + { + name: "secret is nil - git", credType: credentials.TypeGit, helper: &appCredentialHelper{}, assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { @@ -59,6 +77,16 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "no github details provided", + credType: credentials.TypeImage, + secret: &corev1.Secret{}, + helper: &appCredentialHelper{}, + assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { + require.NoError(t, err) + require.Nil(t, creds) + }, + }, + { + name: "no github details provided - git", credType: credentials.TypeGit, secret: &corev1.Secret{}, helper: &appCredentialHelper{}, @@ -69,6 +97,20 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "app ID missing", + credType: credentials.TypeImage, + secret: &corev1.Secret{ + Data: map[string][]byte{ + installationIDKey: []byte(testInstallationIDStr), + privateKeyKey: []byte(testPrivateKey), + }, + }, + helper: &appCredentialHelper{}, + assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { + //require.ErrorContains(t, err, "must all be set or all be unset") + }, + }, + { + name: "app ID missing - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -83,6 +125,20 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "installation ID missing", + credType: credentials.TypeImage, + secret: &corev1.Secret{ + Data: map[string][]byte{ + appIDKey: []byte(testAppIDStr), + privateKeyKey: []byte(testPrivateKey), + }, + }, + helper: &appCredentialHelper{}, + assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { + require.ErrorContains(t, err, "must all be set or all be unset") + }, + }, + { + name: "installation ID missing - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -97,6 +153,20 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "private key missing", + credType: credentials.TypeImage, + secret: &corev1.Secret{ + Data: map[string][]byte{ + appIDKey: []byte(testAppIDStr), + installationIDKey: []byte(testInstallationIDStr), + }, + }, + helper: &appCredentialHelper{}, + assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { + require.ErrorContains(t, err, "must all be set or all be unset") + }, + }, + { + name: "private key missing - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -111,6 +181,26 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "cache hit", + credType: credentials.TypeImage, + secret: &corev1.Secret{ + Data: map[string][]byte{ + appIDKey: []byte(testAppIDStr), + installationIDKey: []byte(testInstallationIDStr), + privateKeyKey: []byte(testPrivateKey), + }, + }, + helper: &appCredentialHelper{ + tokenCache: warmTokenCache, + }, + assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { + require.NoError(t, err) + require.NotNil(t, creds) + require.Equal(t, "kargo", creds.Username) + require.Equal(t, testAccessToken, creds.Password) + }, + }, + { + name: "cache hit - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -131,6 +221,27 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "cache miss; error getting access token", + credType: credentials.TypeImage, + secret: &corev1.Secret{ + Data: map[string][]byte{ + appIDKey: []byte(testAppIDStr), + installationIDKey: []byte(testInstallationIDStr), + privateKeyKey: []byte(testPrivateKey), + }, + }, + helper: &appCredentialHelper{ + tokenCache: cache.New(0, 0), + getAccessTokenFn: func(int64, int64, string) (string, error) { + return "", fmt.Errorf("something went wrong") + }, + }, + assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { + require.ErrorContains(t, err, "error getting installation access token") + require.ErrorContains(t, err, "something went wrong") + }, + }, + { + name: "cache miss; error getting access token - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -152,6 +263,29 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "cache miss; success", + credType: credentials.TypeImage, + secret: &corev1.Secret{ + Data: map[string][]byte{ + appIDKey: []byte(testAppIDStr), + installationIDKey: []byte(testInstallationIDStr), + privateKeyKey: []byte(testPrivateKey), + }, + }, + helper: &appCredentialHelper{ + tokenCache: cache.New(0, 0), + getAccessTokenFn: func(int64, int64, string) (string, error) { + return testAccessToken, nil + }, + }, + assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { + require.NoError(t, err) + require.NotNil(t, creds) + require.Equal(t, "kargo", creds.Username) + require.Equal(t, testAccessToken, creds.Password) + }, + }, + { + name: "cache miss; success - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ From 38f3e0cef65cd8fb024e2390d84b6ec9cf12dc37 Mon Sep 17 00:00:00 2001 From: Dhouib-Mohamed Date: Tue, 6 Aug 2024 19:26:35 +0100 Subject: [PATCH 2/4] feat(tests): removing redundant test cases Signed-off-by: Dhouib-Mohamed --- .../credentials/kubernetes/github/app_test.go | 78 ++++--------------- 1 file changed, 13 insertions(+), 65 deletions(-) diff --git a/internal/credentials/kubernetes/github/app_test.go b/internal/credentials/kubernetes/github/app_test.go index 0aed1d299..625a7a3f1 100644 --- a/internal/credentials/kubernetes/github/app_test.go +++ b/internal/credentials/kubernetes/github/app_test.go @@ -40,8 +40,8 @@ func TestAppCredentialHelper(t *testing.T) { assertions func(*testing.T, *credentials.Credentials, *cache.Cache, error) }{ { - name: "cred type is not git", - credType: credentials.TypeImage, + name: "cred type is not supported", + credType: credentials.TypeHelm, helper: &appCredentialHelper{}, assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { require.NoError(t, err) @@ -58,7 +58,7 @@ func TestAppCredentialHelper(t *testing.T) { }, }, { - name: "secret is nil", + name: "cred type is image", credType: credentials.TypeImage, helper: &appCredentialHelper{}, assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { @@ -67,7 +67,7 @@ func TestAppCredentialHelper(t *testing.T) { }, }, { - name: "secret is nil - git", + name: "secret is nil", credType: credentials.TypeGit, helper: &appCredentialHelper{}, assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { @@ -77,16 +77,6 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "no github details provided", - credType: credentials.TypeImage, - secret: &corev1.Secret{}, - helper: &appCredentialHelper{}, - assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { - require.NoError(t, err) - require.Nil(t, creds) - }, - }, - { - name: "no github details provided - git", credType: credentials.TypeGit, secret: &corev1.Secret{}, helper: &appCredentialHelper{}, @@ -97,20 +87,6 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "app ID missing", - credType: credentials.TypeImage, - secret: &corev1.Secret{ - Data: map[string][]byte{ - installationIDKey: []byte(testInstallationIDStr), - privateKeyKey: []byte(testPrivateKey), - }, - }, - helper: &appCredentialHelper{}, - assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { - //require.ErrorContains(t, err, "must all be set or all be unset") - }, - }, - { - name: "app ID missing - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -125,20 +101,6 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "installation ID missing", - credType: credentials.TypeImage, - secret: &corev1.Secret{ - Data: map[string][]byte{ - appIDKey: []byte(testAppIDStr), - privateKeyKey: []byte(testPrivateKey), - }, - }, - helper: &appCredentialHelper{}, - assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { - require.ErrorContains(t, err, "must all be set or all be unset") - }, - }, - { - name: "installation ID missing - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -153,20 +115,6 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "private key missing", - credType: credentials.TypeImage, - secret: &corev1.Secret{ - Data: map[string][]byte{ - appIDKey: []byte(testAppIDStr), - installationIDKey: []byte(testInstallationIDStr), - }, - }, - helper: &appCredentialHelper{}, - assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { - require.ErrorContains(t, err, "must all be set or all be unset") - }, - }, - { - name: "private key missing - git", credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ @@ -181,7 +129,7 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "cache hit", - credType: credentials.TypeImage, + credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ appIDKey: []byte(testAppIDStr), @@ -200,8 +148,8 @@ func TestAppCredentialHelper(t *testing.T) { }, }, { - name: "cache hit - git", - credType: credentials.TypeGit, + name: "cache hit - image", + credType: credentials.TypeImage, secret: &corev1.Secret{ Data: map[string][]byte{ appIDKey: []byte(testAppIDStr), @@ -221,7 +169,7 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "cache miss; error getting access token", - credType: credentials.TypeImage, + credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ appIDKey: []byte(testAppIDStr), @@ -241,8 +189,8 @@ func TestAppCredentialHelper(t *testing.T) { }, }, { - name: "cache miss; error getting access token - git", - credType: credentials.TypeGit, + name: "cache miss; error getting access token - image", + credType: credentials.TypeImage, secret: &corev1.Secret{ Data: map[string][]byte{ appIDKey: []byte(testAppIDStr), @@ -263,7 +211,7 @@ func TestAppCredentialHelper(t *testing.T) { }, { name: "cache miss; success", - credType: credentials.TypeImage, + credType: credentials.TypeGit, secret: &corev1.Secret{ Data: map[string][]byte{ appIDKey: []byte(testAppIDStr), @@ -285,8 +233,8 @@ func TestAppCredentialHelper(t *testing.T) { }, }, { - name: "cache miss; success - git", - credType: credentials.TypeGit, + name: "cache miss; success - image", + credType: credentials.TypeImage, secret: &corev1.Secret{ Data: map[string][]byte{ appIDKey: []byte(testAppIDStr), From 31c26d8bc6f738eabe8db0c6e7f1689c8387cd93 Mon Sep 17 00:00:00 2001 From: Dhouib-Mohamed Date: Wed, 7 Aug 2024 02:09:11 +0100 Subject: [PATCH 3/4] fix (tests): removing unnecessary tests Signed-off-by: Dhouib-Mohamed --- .../credentials/kubernetes/github/app_test.go | 64 ------------------- 1 file changed, 64 deletions(-) diff --git a/internal/credentials/kubernetes/github/app_test.go b/internal/credentials/kubernetes/github/app_test.go index 625a7a3f1..885c86df1 100644 --- a/internal/credentials/kubernetes/github/app_test.go +++ b/internal/credentials/kubernetes/github/app_test.go @@ -147,26 +147,6 @@ func TestAppCredentialHelper(t *testing.T) { require.Equal(t, testAccessToken, creds.Password) }, }, - { - name: "cache hit - image", - credType: credentials.TypeImage, - secret: &corev1.Secret{ - Data: map[string][]byte{ - appIDKey: []byte(testAppIDStr), - installationIDKey: []byte(testInstallationIDStr), - privateKeyKey: []byte(testPrivateKey), - }, - }, - helper: &appCredentialHelper{ - tokenCache: warmTokenCache, - }, - assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { - require.NoError(t, err) - require.NotNil(t, creds) - require.Equal(t, "kargo", creds.Username) - require.Equal(t, testAccessToken, creds.Password) - }, - }, { name: "cache miss; error getting access token", credType: credentials.TypeGit, @@ -188,27 +168,6 @@ func TestAppCredentialHelper(t *testing.T) { require.ErrorContains(t, err, "something went wrong") }, }, - { - name: "cache miss; error getting access token - image", - credType: credentials.TypeImage, - secret: &corev1.Secret{ - Data: map[string][]byte{ - appIDKey: []byte(testAppIDStr), - installationIDKey: []byte(testInstallationIDStr), - privateKeyKey: []byte(testPrivateKey), - }, - }, - helper: &appCredentialHelper{ - tokenCache: cache.New(0, 0), - getAccessTokenFn: func(int64, int64, string) (string, error) { - return "", fmt.Errorf("something went wrong") - }, - }, - assertions: func(t *testing.T, _ *credentials.Credentials, _ *cache.Cache, err error) { - require.ErrorContains(t, err, "error getting installation access token") - require.ErrorContains(t, err, "something went wrong") - }, - }, { name: "cache miss; success", credType: credentials.TypeGit, @@ -232,29 +191,6 @@ func TestAppCredentialHelper(t *testing.T) { require.Equal(t, testAccessToken, creds.Password) }, }, - { - name: "cache miss; success - image", - credType: credentials.TypeImage, - secret: &corev1.Secret{ - Data: map[string][]byte{ - appIDKey: []byte(testAppIDStr), - installationIDKey: []byte(testInstallationIDStr), - privateKeyKey: []byte(testPrivateKey), - }, - }, - helper: &appCredentialHelper{ - tokenCache: cache.New(0, 0), - getAccessTokenFn: func(int64, int64, string) (string, error) { - return testAccessToken, nil - }, - }, - assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { - require.NoError(t, err) - require.NotNil(t, creds) - require.Equal(t, "kargo", creds.Username) - require.Equal(t, testAccessToken, creds.Password) - }, - }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { From f2a947afefb3bffec3fad6e415acfcf22c60a934 Mon Sep 17 00:00:00 2001 From: Dhouib-Mohamed Date: Wed, 7 Aug 2024 10:48:55 +0100 Subject: [PATCH 4/4] fix (tests): removing unnecessary tests Signed-off-by: Dhouib-Mohamed --- .../credentials/kubernetes/github/app_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/internal/credentials/kubernetes/github/app_test.go b/internal/credentials/kubernetes/github/app_test.go index 885c86df1..57c6dbff9 100644 --- a/internal/credentials/kubernetes/github/app_test.go +++ b/internal/credentials/kubernetes/github/app_test.go @@ -48,24 +48,6 @@ func TestAppCredentialHelper(t *testing.T) { require.Nil(t, creds) }, }, - { - name: "cred type is git", - credType: credentials.TypeGit, - helper: &appCredentialHelper{}, - assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { - require.NoError(t, err) - require.Nil(t, creds) - }, - }, - { - name: "cred type is image", - credType: credentials.TypeImage, - helper: &appCredentialHelper{}, - assertions: func(t *testing.T, creds *credentials.Credentials, _ *cache.Cache, err error) { - require.NoError(t, err) - require.Nil(t, creds) - }, - }, { name: "secret is nil", credType: credentials.TypeGit,