Skip to content

Commit 06408a6

Browse files
onuryilmazonuryilmaz
committed
feat(sync): auth type
1 parent 3941313 commit 06408a6

File tree

3 files changed

+224
-13
lines changed

3 files changed

+224
-13
lines changed

‎cmd/sync.go‎

Lines changed: 104 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"log"
1313
"maps"
14+
"sort"
1415
"strings"
1516

1617
greenhousemetav1alpha1 "github.com/cloudoperators/greenhouse/api/meta/v1alpha1"
@@ -30,6 +31,9 @@ var (
3031
remoteClusterName string
3132
prefix string
3233
mergeIdenticalUsers bool
34+
authType string
35+
kubeloginPath string
36+
kubeloginExtraArgs []string
3337
)
3438

3539
func init() {
@@ -41,6 +45,11 @@ func init() {
4145
syncCmd.Flags().StringVar(&remoteClusterName, "remote-cluster-name", "", "name of the remote cluster, if not set all clusters are retrieved")
4246
syncCmd.Flags().StringVar(&prefix, "prefix", "cloudctl", "prefix for kubeconfig entries. it is used to separate and manage the entries of this tool only")
4347
syncCmd.Flags().BoolVar(&mergeIdenticalUsers, "merge-identical-users", true, "merge identical user information in kubeconfig file so that you only login once for the clusters that share the same auth info")
48+
49+
// Authentication output style flags
50+
syncCmd.Flags().StringVar(&authType, "auth-type", "auth-provider", "authentication config style to write for users: auth-provider or exec-plugin")
51+
syncCmd.Flags().StringVar(&kubeloginPath, "kubelogin-path", "kubelogin", "path to kubelogin command when using exec-plugin auth-type")
52+
syncCmd.Flags().StringSliceVar(&kubeloginExtraArgs, "kubelogin-extra-args", nil, "extra arguments to pass to kubelogin exec plugin")
4453
}
4554

4655
var syncCmd = &cobra.Command{
@@ -136,7 +145,7 @@ func filterReady(items []v1alpha1.ClusterKubeconfig) []v1alpha1.ClusterKubeconfi
136145
eligible := make([]v1alpha1.ClusterKubeconfig, 0, len(items))
137146
for _, ckc := range items {
138147
cond := ckc.Status.Conditions.GetConditionByType(greenhousemetav1alpha1.ReadyCondition)
139-
if cond == nil || cond.IsTrue() {
148+
if cond != nil && cond.IsTrue() {
140149
eligible = append(eligible, ckc)
141150
}
142151
}
@@ -160,12 +169,26 @@ func buildIncomingKubeconfig(items []v1alpha1.ClusterKubeconfig) (*clientcmdapi.
160169

161170
// Add all users (auth infos)
162171
for _, authItem := range ckc.Spec.Kubeconfig.AuthInfo {
163-
// Preserve the same data shape; exclude nothing here (merging will handle dedupe)
164-
authProvider := authItem.AuthInfo.AuthProvider
165-
kubeconfig.AuthInfos[authItem.Name] = &clientcmdapi.AuthInfo{
166-
ClientCertificateData: authItem.AuthInfo.ClientCertificateData,
167-
ClientKeyData: authItem.AuthInfo.ClientKeyData,
168-
AuthProvider: &authProvider,
172+
// Depending on the selected auth type, keep legacy auth-provider or convert to exec plugin
173+
if strings.EqualFold(authType, "exec-plugin") && authItem.AuthInfo.AuthProvider.Name == "oidc" {
174+
execAuth := &clientcmdapi.AuthInfo{
175+
ClientCertificateData: authItem.AuthInfo.ClientCertificateData,
176+
ClientKeyData: authItem.AuthInfo.ClientKeyData,
177+
Exec: &clientcmdapi.ExecConfig{
178+
APIVersion: "client.authentication.k8s.io/v1",
179+
Command: kubeloginPath,
180+
Args: buildKubeloginArgs(authItem.AuthInfo.AuthProvider.Config, kubeloginExtraArgs),
181+
InteractiveMode: clientcmdapi.IfAvailableExecInteractiveMode,
182+
},
183+
}
184+
kubeconfig.AuthInfos[authItem.Name] = execAuth
185+
} else {
186+
// Preserve the same data shape; exclude nothing here (merging will handle dedupe)
187+
kubeconfig.AuthInfos[authItem.Name] = &clientcmdapi.AuthInfo{
188+
ClientCertificateData: authItem.AuthInfo.ClientCertificateData,
189+
ClientKeyData: authItem.AuthInfo.ClientKeyData,
190+
AuthProvider: &authItem.AuthInfo.AuthProvider,
191+
}
169192
}
170193
}
171194

@@ -230,8 +253,27 @@ func authInfoEqual(a, b *clientcmdapi.AuthInfo) bool {
230253
return false
231254
}
232255

256+
// Compare Exec first (new style)
257+
if (a.Exec == nil) != (b.Exec == nil) {
258+
return false
259+
}
260+
if a.Exec != nil && b.Exec != nil {
261+
if a.Exec.Command != b.Exec.Command || a.Exec.APIVersion != b.Exec.APIVersion {
262+
return false
263+
}
264+
if len(a.Exec.Args) != len(b.Exec.Args) {
265+
return false
266+
}
267+
for i := range a.Exec.Args {
268+
if a.Exec.Args[i] != b.Exec.Args[i] {
269+
return false
270+
}
271+
}
272+
return true
273+
}
274+
233275
// Compare AuthProvider, excluding "id-token" and "refresh-token"
234-
if a.AuthProvider == nil && b.AuthProvider != nil || a.AuthProvider != nil && b.AuthProvider == nil {
276+
if (a.AuthProvider == nil) != (b.AuthProvider == nil) {
235277
return false
236278
}
237279
if a.AuthProvider != nil && b.AuthProvider != nil {
@@ -266,6 +308,30 @@ func filterAuthProviderConfig(config map[string]string) map[string]string {
266308
// excluding "id-token" and "refresh-token". It uses "client-id", "client-secret",
267309
// "auth-request-extra-params", and "extra-scopes" to generate the key.
268310
func generateAuthInfoKey(authInfo *clientcmdapi.AuthInfo) string {
311+
// Exec-based key: derive from stable subset of args to avoid including tokens
312+
if authInfo.Exec != nil {
313+
// Extract known kubelogin flags
314+
var issuer, clientID, clientSecret, extraParams string
315+
var scopes []string
316+
for _, arg := range authInfo.Exec.Args {
317+
switch {
318+
case strings.HasPrefix(arg, "--oidc-issuer-url="):
319+
issuer = strings.TrimPrefix(arg, "--oidc-issuer-url=")
320+
case strings.HasPrefix(arg, "--oidc-client-id="):
321+
clientID = strings.TrimPrefix(arg, "--oidc-client-id=")
322+
case strings.HasPrefix(arg, "--oidc-client-secret="):
323+
clientSecret = strings.TrimPrefix(arg, "--oidc-client-secret=")
324+
case strings.HasPrefix(arg, "--oidc-extra-scope="):
325+
scopes = append(scopes, strings.TrimPrefix(arg, "--oidc-extra-scope="))
326+
case strings.HasPrefix(arg, "--oidc-auth-request-extra-params="):
327+
extraParams = strings.TrimPrefix(arg, "--oidc-auth-request-extra-params=")
328+
}
329+
}
330+
sort.Strings(scopes)
331+
data := fmt.Sprintf("exec:issuer:%s;client-id:%s;client-secret:%s;extra-params:%s;scopes:%s", issuer, clientID, clientSecret, extraParams, strings.Join(scopes, ","))
332+
return data
333+
}
334+
269335
if authInfo.AuthProvider == nil {
270336
// For AuthInfos without AuthProvider, use a different unique identifier
271337
// Here, we'll use the hash of ClientCertificateData and ClientKeyData
@@ -288,6 +354,36 @@ func generateAuthInfoKey(authInfo *clientcmdapi.AuthInfo) string {
288354
return data
289355
}
290356

357+
// buildKubeloginArgs constructs kubelogin arguments from an oidc auth-provider config and extra args
358+
func buildKubeloginArgs(cfg map[string]string, extra []string) []string {
359+
args := []string{"get-token"}
360+
if v := cfg["idp-issuer-url"]; v != "" {
361+
args = append(args, "--oidc-issuer-url="+v)
362+
}
363+
if v := cfg["client-id"]; v != "" {
364+
args = append(args, "--oidc-client-id="+v)
365+
}
366+
if v := cfg["client-secret"]; v != "" {
367+
args = append(args, "--oidc-client-secret="+v)
368+
}
369+
if v := cfg["extra-scopes"]; v != "" {
370+
for _, s := range strings.Split(v, ",") {
371+
s = strings.TrimSpace(s)
372+
if s != "" {
373+
args = append(args, "--oidc-extra-scope="+s)
374+
}
375+
}
376+
}
377+
if v := cfg["auth-request-extra-params"]; v != "" {
378+
args = append(args, "--oidc-auth-request-extra-params="+v)
379+
}
380+
// allow caller to inject additional flags
381+
if len(extra) > 0 {
382+
args = append(args, extra...)
383+
}
384+
return args
385+
}
386+
291387
func mergeKubeconfig(localConfig *clientcmdapi.Config, serverConfig *clientcmdapi.Config) error {
292388
// Merge Clusters
293389
for serverName, serverCluster := range serverConfig.Clusters {

‎cmd/sync_test.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,3 +207,111 @@ func TestFilterReady_EmptyAndNoneReady(t *testing.T) {
207207
out2 := filterReady([]greenhousev1alpha1.ClusterKubeconfig{a, b})
208208
g.Expect(out2).To(HaveLen(0))
209209
}
210+
211+
// ---- New tests for exec-plugin flags and helpers ----
212+
213+
func TestSyncFlags_AuthTypeAndKubeloginDefaults(t *testing.T) {
214+
g := NewWithT(t)
215+
216+
// Ensure flags are registered on syncCmd with correct defaults
217+
fAuthType := syncCmd.Flags().Lookup("auth-type")
218+
g.Expect(fAuthType).ToNot(BeNil())
219+
g.Expect(fAuthType.DefValue).To(Equal("auth-provider"))
220+
221+
fPath := syncCmd.Flags().Lookup("kubelogin-path")
222+
g.Expect(fPath).ToNot(BeNil())
223+
g.Expect(fPath.DefValue).To(Equal("kubelogin"))
224+
225+
fExtra := syncCmd.Flags().Lookup("kubelogin-extra-args")
226+
g.Expect(fExtra).ToNot(BeNil())
227+
// StringSliceVar defaults to [] if nil; DefValue is representation of default (empty)
228+
g.Expect(fExtra.DefValue).To(Or(Equal("[]"), Equal("")))
229+
}
230+
231+
func TestBuildKubeloginArgs_MappingAndExtras(t *testing.T) {
232+
g := NewWithT(t)
233+
234+
cfg := map[string]string{
235+
"idp-issuer-url": "https://issuer.example.com",
236+
"client-id": "cid",
237+
"client-secret": "csec",
238+
"extra-scopes": "groups, offline_access ,email",
239+
"auth-request-extra-params": "aud=foo,foo=bar",
240+
}
241+
extra := []string{"--v=4", "--token-cache-dir=/tmp/k"}
242+
243+
args := buildKubeloginArgs(cfg, extra)
244+
245+
// Starts with subcommand
246+
g.Expect(args[0]).To(Equal("get-token"))
247+
// Contains mapped flags
248+
g.Expect(args).To(ContainElement("--oidc-issuer-url=https://issuer.example.com"))
249+
g.Expect(args).To(ContainElement("--oidc-client-id=cid"))
250+
g.Expect(args).To(ContainElement("--oidc-client-secret=csec"))
251+
// Each scope becomes separate flag; whitespace trimmed
252+
g.Expect(args).To(ContainElements(
253+
"--oidc-extra-scope=groups",
254+
"--oidc-extra-scope=offline_access",
255+
"--oidc-extra-scope=email",
256+
))
257+
// Extra params
258+
g.Expect(args).To(ContainElement("--oidc-auth-request-extra-params=aud=foo,foo=bar"))
259+
// Extra args appended
260+
g.Expect(args[len(args)-2:]).To(Equal(extra))
261+
}
262+
263+
func TestAuthInfoEqual_ExecBased(t *testing.T) {
264+
g := NewWithT(t)
265+
266+
baseArgs := []string{"get-token", "--oidc-issuer-url=x", "--oidc-client-id=a"}
267+
a := &clientcmdapi.AuthInfo{Exec: &clientcmdapi.ExecConfig{APIVersion: "client.authentication.k8s.io/v1", Command: "kubelogin", Args: append([]string{}, baseArgs...)}}
268+
b := &clientcmdapi.AuthInfo{Exec: &clientcmdapi.ExecConfig{APIVersion: "client.authentication.k8s.io/v1", Command: "kubelogin", Args: append([]string{}, baseArgs...)}}
269+
g.Expect(authInfoEqual(a, b)).To(BeTrue())
270+
271+
// Change an arg should make them different
272+
b.Exec.Args[2] = "--oidc-client-id=DIFF"
273+
g.Expect(authInfoEqual(a, b)).To(BeFalse())
274+
275+
// Change command should make them different
276+
b = b.DeepCopy()
277+
b.Exec.Command = "other"
278+
g.Expect(authInfoEqual(a, b)).To(BeFalse())
279+
}
280+
281+
func TestGenerateAuthInfoKey_ExecStableIgnoresOrder(t *testing.T) {
282+
g := NewWithT(t)
283+
284+
// Same effective parameters but different order of scope flags
285+
a := &clientcmdapi.AuthInfo{Exec: &clientcmdapi.ExecConfig{
286+
APIVersion: "client.authentication.k8s.io/v1",
287+
Command: "kubelogin",
288+
Args: []string{
289+
"get-token",
290+
"--oidc-issuer-url=https://issuer",
291+
"--oidc-client-id=cid",
292+
"--oidc-client-secret=csec",
293+
"--oidc-auth-request-extra-params=aud=foo",
294+
"--oidc-extra-scope=email",
295+
"--oidc-extra-scope=groups",
296+
},
297+
}}
298+
299+
b := &clientcmdapi.AuthInfo{Exec: &clientcmdapi.ExecConfig{
300+
APIVersion: "client.authentication.k8s.io/v1",
301+
Command: "kubelogin",
302+
Args: []string{
303+
"get-token",
304+
"--oidc-extra-scope=groups",
305+
"--oidc-issuer-url=https://issuer",
306+
"--oidc-client-id=cid",
307+
"--oidc-client-secret=csec",
308+
"--oidc-extra-scope=email",
309+
"--oidc-auth-request-extra-params=aud=foo",
310+
"--v=4", // unrelated extra should not affect extracted key fields
311+
},
312+
}}
313+
314+
ka := generateAuthInfoKey(a)
315+
kb := generateAuthInfoKey(b)
316+
g.Expect(ka).To(Equal(kb))
317+
}

‎e2e/sync_test.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,12 @@ spec:
8989
cluster: rc
9090
user: user
9191
namespace: default
92+
status:
93+
statusConditions:
94+
conditions:
95+
- reason: Complete
96+
status: "True"
97+
type: Ready
9298
`
9399
writeFile(t, crFile, crYAML)
94100

@@ -104,20 +110,21 @@ spec:
104110
t.Fatalf("apply CR failed: %v (stderr: %s)", err, stderr)
105111
}
106112

107-
// Since cloudctl sync only considers ClusterKubeconfigs with Ready=True,
108-
// patch the status of our demo resource accordingly and wait until it's reflected.
109-
// Note: We don't have a controller in this e2e setup, so we set the status manually.
113+
// Explicitly patch status to Ready=True via the status subresource so the sync can filter by readiness
114+
// This mirrors what a controller would normally set, but keeps the e2e test self-contained.
115+
// Note: status.statusConditions is an object with a nested 'conditions' list in this CRD and requires lastTransitionTime.
116+
now := time.Now().UTC().Format(time.RFC3339Nano)
117+
patch := fmt.Sprintf(`{"status":{"statusConditions":{"conditions":[{"type":"Ready","status":"True","reason":"E2E","message":"ready","lastTransitionTime":"%s"}]}}}`, now)
110118
if _, stderr, err := runCmd(
111119
"kubectl", "--kubeconfig", kubeconfig,
112120
"-n", ns,
113121
"patch", "clusterkubeconfig", "demo",
114122
"--type", "merge",
115123
"--subresource", "status",
116-
"-p", `{"status":{"conditions":[{"type":"Ready","status":"True","reason":"E2E","message":"ready"}]}}`,
124+
"-p", patch,
117125
); err != nil {
118126
t.Fatalf("patch status failed: %v (stderr: %s)", err, stderr)
119127
}
120-
121128
// Target kubeconfig file
122129
targetKubeconfig := filepath.Join(os.TempDir(), "e2e-sync-target-kubeconfig")
123130
createEmptyKubeconfigFile(t, targetKubeconfig)

0 commit comments

Comments
 (0)