Skip to content

Commit

Permalink
Make principals dynamic in Profiles Controller (#7310) (#7336)
Browse files Browse the repository at this point in the history
* Make principals dynamic in Profiles Controller



* review: Use dynamic principal of IGW in KFAM



* review: Add env vars to manifests



* review: Add pipelines-ui principal



* Update golang to 1.19 for unit tests to succeed



* review: Include KFP UI principle in profiles/kfam



---------

Signed-off-by: Kimonas Sotirchos <kimwnasptd@gmail.com>
Co-authored-by: Kimonas Sotirchos <kimwnasptd@gmail.com>
  • Loading branch information
DnPlas and kimwnasptd committed Oct 13, 2023
1 parent f33d55c commit e28be5d
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/prof_controller_unit_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: Install Go
uses: actions/setup-go@v3
with:
go-version: '1.17'
go-version: '1.19'
check-latest: true

- name: Run unit tests
Expand Down
11 changes: 10 additions & 1 deletion components/access-management/kfam/bindings.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ func getBindingName(binding *Binding) (string, error) {
}

func getAuthorizationPolicy(binding *Binding, userIdHeader string, userIdPrefix string) istioSecurity.AuthorizationPolicy {
istioIGWPrincipal := GetEnvDefault(
"ISTIO_INGRESS_GATEWAY_PRINCIPAL",
"cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account")

kfpUIPrincipal := GetEnvDefault(
"KFP_UI_PRINCIPAL",
"cluster.local/ns/kubeflow/sa/ml-pipeline-ui")

return istioSecurity.AuthorizationPolicy{
Rules: []*istioSecurity.Rule{
{
Expand All @@ -91,7 +99,8 @@ func getAuthorizationPolicy(binding *Binding, userIdHeader string, userIdPrefix
From: []*istioSecurity.Rule_From{{
Source: &istioSecurity.Source{
Principals: []string{
"cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account",
istioIGWPrincipal,
kfpUIPrincipal,
},
},
}},
Expand Down
10 changes: 10 additions & 0 deletions components/access-management/kfam/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,13 @@
// limitations under the License.

package kfam

import "os"

func GetEnvDefault(variable string, defaultVal string) string {
envVar := os.Getenv(variable)
if len(envVar) == 0 {
return defaultVal
}
return envVar
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,7 @@ configMapGenerator:
- WORKLOAD_IDENTITY=
- USERID_HEADER="kubeflow-userid"
- USERID_PREFIX=
name: config
- ISTIO_INGRESS_GATEWAY_PRINCIPAL="cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account")
- NOTEBOOK_CONTROLLER_PRINCIPAL="cluster.local/ns/kubeflow/sa/notebook-controller-service-account")
- KFP_UI_PRINCIPAL="cluster.local/ns/kubeflow/sa/ml-pipeline-ui"
name: config
28 changes: 21 additions & 7 deletions components/profile-controller/controllers/profile_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,17 @@ func (r *ProfileReconciler) SetupWithManager(mgr ctrl.Manager) error {
}

func (r *ProfileReconciler) getAuthorizationPolicy(profileIns *profilev1.Profile) istioSecurity.AuthorizationPolicy {
nbControllerPrincipal := GetEnvDefault(
"NOTEBOOK_CONTROLLER_PRINCIPAL",
"cluster.local/ns/kubeflow/sa/notebook-controller-service-account")

clusterDomain := "cluster.local"
if clusterDomainFromEnv, ok := os.LookupEnv("CLUSTER_DOMAIN"); ok {
clusterDomain = clusterDomainFromEnv
}
principals := fmt.Sprintf("%s/ns/kubeflow/sa/notebook-controller-service-account", clusterDomain)
istioIGWPrincipal := GetEnvDefault(
"ISTIO_INGRESS_GATEWAY_PRINCIPAL",
"cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account")

kfpUIPrincipal := GetEnvDefault(
"KFP_UI_PRINCIPAL",
"cluster.local/ns/kubeflow/sa/ml-pipeline-ui")

return istioSecurity.AuthorizationPolicy{
Action: istioSecurity.AuthorizationPolicy_ALLOW,
Expand All @@ -443,7 +448,8 @@ func (r *ProfileReconciler) getAuthorizationPolicy(profileIns *profilev1.Profile
From: []*istioSecurity.Rule_From{{
Source: &istioSecurity.Source{
Principals: []string{
"cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account",
istioIGWPrincipal,
kfpUIPrincipal,
},
},
}},
Expand Down Expand Up @@ -480,7 +486,7 @@ func (r *ProfileReconciler) getAuthorizationPolicy(profileIns *profilev1.Profile
From: []*istioSecurity.Rule_From{
{
Source: &istioSecurity.Source{
Principals: []string{principals},
Principals: []string{nbControllerPrincipal},
},
},
},
Expand Down Expand Up @@ -782,3 +788,11 @@ func (r *ProfileReconciler) readDefaultLabelsFromFile(path string) map[string]st
}
return labels
}

func GetEnvDefault(variable string, defaultVal string) string {
envVar := os.Getenv(variable)
if len(envVar) == 0 {
return defaultVal
}
return envVar
}

0 comments on commit e28be5d

Please sign in to comment.