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

fix!: remove list and watch on secrets. Fixes #8534 #8555

Merged
merged 23 commits into from
May 13, 2022
Merged

fix!: remove list and watch on secrets. Fixes #8534 #8555

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e6afd37
fix: remove secrets
Apr 30, 2022
c06ea3e
feat: remove list and watch on secrets
Apr 30, 2022
65ac3e9
fix: lint
May 1, 2022
e096ce8
feat: initial draft of timed cache
May 2, 2022
fa5a098
feat: initial draft of timed cache
May 2, 2022
7a56851
feat: merge
May 2, 2022
ada3ef4
feat: add tests
May 2, 2022
c8772ec
feat: merge
May 3, 2022
41830bd
fix: remove generics to fix build
May 3, 2022
83ed08b
feat: rename variables
May 3, 2022
1155102
feat: rename variables
May 3, 2022
901ea65
feat: rename variables
May 3, 2022
8b0b362
fix: remove sso-namespace option
May 4, 2022
56e064c
fix: set cache TTL to 1m
May 4, 2022
0e57a32
fix: docs
May 4, 2022
22e6bdb
feat: fix namespaces
May 4, 2022
17a649f
fix: rename variable
May 4, 2022
629f3dd
fix: minor
May 5, 2022
edc69b3
feat: start informerFactory in a separate method
May 5, 2022
56fbaaf
feat: fix time tests
May 5, 2022
e232df4
fix: resolve ssonamespace in
May 5, 2022
91271eb
fix: pass ctx as function argument
May 6, 2022
d0541c1
fix: pass ctx as function argument
May 6, 2022
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: 7 additions & 0 deletions cmd/argo/commands/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,16 @@ See %s`, help.ArgoServer),
managedNamespace = namespace
}

ssoNamespace := namespace
if managedNamespace != "" {
ssoNamespace = managedNamespace
}
Comment on lines +100 to +103
Copy link

@agilgur5 agilgur5 Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this actually is different from the logic on line 151 and line 160 below. Those set the SSO namespace to the installation namespace when using a managed namespace. That makes sense, since your Argo configuration and Argo Server are in the installation namespace. Only your Workflows are in the managed namespace.

This change is actually quite confusing as a result, as due to this, with a managed namespace you now configure SSO RBAC there and not in the installation namespace. Normally that only happens when you enable SSO namespace delegation, which is named fairly explicitly.

This resulted in an undocumented breaking change that caused a regression too: #9989 (comment) 😕


log.WithFields(log.Fields{
"authModes": authModes,
"namespace": namespace,
"managedNamespace": managedNamespace,
"ssoNamespace": ssoNamespace,
"baseHRef": baseHRef,
"secure": secure,
}).Info()
Expand Down Expand Up @@ -152,6 +158,7 @@ See %s`, help.ArgoServer),
RestConfig: config,
AuthModes: modes,
ManagedNamespace: managedNamespace,
SSONamespace: ssoNamespace,
alexec marked this conversation as resolved.
Show resolved Hide resolved
ConfigName: configMap,
EventOperationQueueSize: eventOperationQueueSize,
EventWorkerCount: eventWorkerCount,
Expand Down
10 changes: 2 additions & 8 deletions server/apiserver/argoserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ type ArgoServerOpts struct {
// config map name
ConfigName string
ManagedNamespace string
SSONamespace string
HSTS bool
EventOperationQueueSize int
EventWorkerCount int
Expand All @@ -115,13 +116,6 @@ func init() {
}
}

func getSSONamespace(opts ArgoServerOpts) string {
if opts.ManagedNamespace != "" {
return opts.ManagedNamespace
}
return opts.Namespace
}

func getResourceCacheNamespace(opts ArgoServerOpts) string {
if opts.ManagedNamespace != "" {
return opts.ManagedNamespace
Expand All @@ -148,7 +142,7 @@ func NewArgoServer(ctx context.Context, opts ArgoServerOpts) (*argoServer, error
} else {
log.Info("SSO disabled")
}
gatekeeper, err := auth.NewGatekeeper(opts.AuthModes, opts.Clients, opts.RestConfig, ssoIf, auth.DefaultClientForAuthorization, opts.Namespace, getSSONamespace(opts), opts.Namespaced, resourceCache)
gatekeeper, err := auth.NewGatekeeper(opts.AuthModes, opts.Clients, opts.RestConfig, ssoIf, auth.DefaultClientForAuthorization, opts.Namespace, opts.SSONamespace, opts.Namespaced, resourceCache)
if err != nil {
return nil, err
}
Expand Down