Skip to content

Commit

Permalink
Remove Graph dependency from pipeline principal
Browse files Browse the repository at this point in the history
  • Loading branch information
johnstairs committed Oct 4, 2023
1 parent 744e328 commit 8fd5cd3
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 50 deletions.
1 change: 0 additions & 1 deletion .devcontainer/update-devcontainer-image.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

set -euo pipefail

azure_subscription="BiomedicalImaging-NonProd"
devcontainer_registry_subdomain="compimagdevcontainers"
devcontainer_registry="${devcontainer_registry_subdomain}.azurecr.io"
devcontainer_repository="${devcontainer_registry}/tyger"
Expand Down
2 changes: 1 addition & 1 deletion cli/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ require (
github.com/spf13/cobra v1.7.0
github.com/stretchr/testify v1.8.4
go.opentelemetry.io/otel v1.19.0
helm.sh/helm/v3 v3.13.0
helm.sh/helm/v3 v3.12.3
k8s.io/api v0.28.2
k8s.io/apimachinery v0.28.2
k8s.io/client-go v0.28.2
Expand Down
4 changes: 2 additions & 2 deletions cli/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -709,8 +709,8 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o=
gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g=
helm.sh/helm/v3 v3.13.0 h1:XPJKIU30K4JTQ6VX/6e0hFAmEIonYa8E7wx5aqv4xOc=
helm.sh/helm/v3 v3.13.0/go.mod h1:2PBEKsMWKLVZTojUOqMS3Eadv5mP43FBWrRgLNkNm9Y=
helm.sh/helm/v3 v3.12.3 h1:5y1+Sbty12t48T/t/CGNYUIME5BJ0WKfmW/sobYqkFg=
helm.sh/helm/v3 v3.12.3/go.mod h1:KPKQiX9IP5HX7o5YnnhViMnNuKiL/lJBVQ47GHe1R0k=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
k8s.io/api v0.28.2 h1:9mpl5mOb6vXZvqbQmankOfPIGiudghwCoLl1EYfUZbw=
k8s.io/api v0.28.2/go.mod h1:RVnJBsjU8tcMq7C3iaRSGMeaKt2TWEUXcpIt/90fjEg=
Expand Down
24 changes: 12 additions & 12 deletions cli/internal/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func newConfigCreateCommand() *cobra.Command {

for {
for {
templateValues.CurrentUserId, templateValues.CurrentUserDisplayName, err = getCurrentPrincipal(cred)
templateValues.PrincipalId, templateValues.PrincipalDisplayName, templateValues.PrincipalKind, err = getCurrentPrincipal(cred)
if err != nil {
if err == errNotLoggedIn {
fmt.Printf("You are not logged in to Azure. Please run `az login` in another terminal window.\nPress any key to continue when ready...\n\n")
Expand All @@ -116,7 +116,7 @@ func newConfigCreateCommand() *cobra.Command {
break
}

input := confirmation.New(fmt.Sprintf("You are logged in as %s. Is that the right account?", templateValues.CurrentUserDisplayName), confirmation.Yes)
input := confirmation.New(fmt.Sprintf("You are logged in as %s. Is that the right account?", templateValues.PrincipalDisplayName), confirmation.Yes)
ready, err := input.RunPrompt()
if err != nil {
return err
Expand Down Expand Up @@ -306,42 +306,42 @@ func chooseSubscription(cred azcore.TokenCredential) (string, error) {
return sub.id, nil
}

func getCurrentPrincipal(cred azcore.TokenCredential) (oid, display string, err error) {
func getCurrentPrincipal(cred azcore.TokenCredential) (oid, display string, kind install.PrincipalKind, err error) {
tokenResponse, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{cloud.AzurePublic.Services[cloud.ResourceManager].Audience}})
if err != nil {
return "", "", errNotLoggedIn
return "", "", "", errNotLoggedIn
}

claims := jwt.MapClaims{}
_, _, err = jwt.NewParser().ParseUnverified(tokenResponse.Token, claims)
if err != nil {
return "", "", fmt.Errorf("failed to parse token: %w", err)
return "", "", "", fmt.Errorf("failed to parse token: %w", err)
}
oid = claims["oid"].(string)
if unique_name, ok := claims["unique_name"]; ok {
display = unique_name.(string)
return oid, display, nil
return oid, display, install.PrincipalKindUser, nil
}

principals, err := install.ObjectsIdToPrincipals(context.Background(), []string{oid})
if err != nil {
return "", "", err
return "", "", "", err
}

if len(principals) == 0 {
return "", "", errors.New("principal not found")
return "", "", "", errors.New("principal not found")
}

if principals[0].Kind != install.PrincipalKindServicePrincipal {
return "", "", errors.New("principal is not a service principal")
return "", "", "", errors.New("principal was expected to be a service principal but isn't")
}

display, err = install.GetServicePrincipalDisplayName(context.Background(), principals[0].Id)
display, err = install.GetServicePrincipalDisplayName(context.Background(), principals[0].ObjectId)
if err != nil {
return "", "", err
return "", "", "", err
}

return oid, display, nil
return oid, display, install.PrincipalKindServicePrincipal, nil
}

func chooseTenant(cred azcore.TokenCredential, prompt string, presentOtherOption bool) (string, error) {
Expand Down
20 changes: 17 additions & 3 deletions cli/internal/install/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,23 @@ type CloudConfig struct {

type ComputeConfig struct {
Clusters []*ClusterConfig `json:"clusters"`
ManagementPrincipalIds []string `json:"managementPrincipalIds"`
ManagementPrincipals []Principal `json:"managementPrincipals"`
PrivateContainerRegistries []string `json:"privateContainerRegistries"`
}

type PrincipalKind string

const (
PrincipalKindUser PrincipalKind = "User"
PrincipalKindGroup PrincipalKind = "Group"
PrincipalKindServicePrincipal PrincipalKind = "ServicePrincipal"
)

type Principal struct {
ObjectId string `json:"objectId"`
Kind PrincipalKind `json:"kind"`
}

func (c *ComputeConfig) GetApiHostCluster() *ClusterConfig {
for _, c := range c.Clusters {
if c.ApiHost {
Expand Down Expand Up @@ -99,8 +112,9 @@ type ConfigTemplateValues struct {
TenantId string
SubscriptionId string
DefaultLocation string
CurrentUserId string
CurrentUserDisplayName string
PrincipalId string
PrincipalDisplayName string
PrincipalKind PrincipalKind
BufferStorageAccountName string
LogsStorageAccountName string
DomainName string
Expand Down
5 changes: 3 additions & 2 deletions cli/internal/install/config.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ cloud:
# These are the principals that will be granted full access to the
# "tyger" namespace in each cluster.
managementPrincipalIds:
- {{ .CurrentUserId }} # {{ .CurrentUserDisplayName }}
- objectId: {{ .CurrentUserId }} # {{ .CurrentUserDisplayName }}
type: {{ .PrincipalKind }}

# Specify the names of private container registries that the clusters must be able to pull from
# The names of private container registries that the clusters must be able to pull from
# privateContainerRegistries:
# - myprivateregistry

Expand Down
17 changes: 2 additions & 15 deletions cli/internal/install/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,8 @@ import (
"github.com/rs/zerolog/log"
)

type PrincipalKind string

const (
PrincipalKindUser PrincipalKind = "User"
PrincipalKindGroup PrincipalKind = "Group"
PrincipalKindServicePrincipal PrincipalKind = "ServicePrincipal"
)

var errNotFound = fmt.Errorf("not found")

type Principal struct {
Id string
Kind PrincipalKind
}

func GetGraphToken(ctx context.Context) (azcore.AccessToken, error) {
cred := GetAzureCredentialFromContext(ctx)
tokenResponse, err := cred.GetToken(ctx, policy.TokenRequestOptions{
Expand Down Expand Up @@ -163,8 +150,8 @@ func ObjectsIdToPrincipals(ctx context.Context, objectIds []string) ([]Principal
}

principal = &Principal{
Id: value.Id,
Kind: kind,
ObjectId: value.Id,
Kind: kind,
}
break
}
Expand Down
11 changes: 2 additions & 9 deletions cli/internal/install/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@ func createTygerNamespace(ctx context.Context, restConfigPromise *Promise[*rest.

func createTygerClusterRBAC(ctx context.Context, restConfigPromise *Promise[*rest.Config], createTygerNamespacePromise *Promise[any]) (any, error) {
config := GetConfigFromContext(ctx)

// we can fetch the principals from the Graph API while we wait for the cluster to be ready
principals, err := ObjectsIdToPrincipals(ctx, config.Cloud.Compute.ManagementPrincipalIds)
if err != nil {
return nil, err
}

restConfig, err := restConfigPromise.Await()
if err != nil {
return nil, errDependencyFailed
Expand Down Expand Up @@ -85,9 +78,9 @@ func createTygerClusterRBAC(ctx context.Context, restConfigPromise *Promise[*res
Subjects: make([]rbacv1.Subject, 0),
}

for _, principal := range principals {
for _, principal := range config.Cloud.Compute.ManagementPrincipals {
subject := rbacv1.Subject{
Name: principal.Id,
Name: principal.ObjectId,
Namespace: TygerNamespace,
}
if principal.Kind == PrincipalKindGroup {
Expand Down
14 changes: 14 additions & 0 deletions cli/internal/install/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,20 @@ func quickValidateComputeConfig(success *bool, cloudConfig *CloudConfig) {
if !hasApiHost {
validationError(success, "One cluster must have `apiHost` set to true")
}

if len(computeConfig.ManagementPrincipals) == 0 {
validationError(success, "At least one management principal is required")
}

for _, p := range computeConfig.ManagementPrincipals {
switch p.Kind {
case PrincipalKindUser, PrincipalKindGroup, PrincipalKindServicePrincipal:
case "":
validationError(success, "The `kind` field is required on a management principal")
default:
validationError(success, "The `kind` field must be one of %v", []PrincipalKind{PrincipalKindUser, PrincipalKindGroup, PrincipalKindServicePrincipal})
}
}
}

func quickValidateStorageConfig(success *bool, cloudConfig *CloudConfig) {
Expand Down
2 changes: 1 addition & 1 deletion deploy/config/microsoft/ci/scaled.cue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package tyger

#NodePoolConfig: {
minCount: *1 | int
minCount: *1 | int
}
12 changes: 9 additions & 3 deletions deploy/config/microsoft/microsoft.cue
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,15 @@ config: #EnvironmentConfig & {
]
},
]
managementPrincipalIds: [
"5b60f594-a0eb-410c-a3fc-dd3c6f4e28d1",
"c0e60aba-35f0-4778-bc9b-fc5d2af14687",
managementPrincipals: [
{
objectId: "5b60f594-a0eb-410c-a3fc-dd3c6f4e28d1"
kind: "ServicePrincipal"
},
{
objectId: "c0e60aba-35f0-4778-bc9b-fc5d2af14687"
kind: "Group"
},
]
privateContainerRegistries: [developerConfig.wipContainerRegistry.name]
}
Expand Down
7 changes: 6 additions & 1 deletion deploy/config/schema.cue
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,14 @@ import "strings"
storage!: #StorageConfig
}

#Principal: {
objectId!: string
kind!: "User" | "Group" | "ServicePrincipal"
}

#ComputeConfig: {
clusters!: [...#ClusterConfig]
managementPrincipalIds?: [...string]
managementPrincipals?: [...#Principal]
privateContainerRegistries?: [...string]
}

Expand Down

0 comments on commit 8fd5cd3

Please sign in to comment.