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.

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

Added SDK calls for createAzApp #276

Closed
wants to merge 10 commits into from
2 changes: 1 addition & 1 deletion cmd/setup-gh.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func createGraphClient(azCred *azidentity.DefaultAzureCredential) (providers.Gra
if err != nil {
return nil, fmt.Errorf("creating graph service client: %w", err)
}
return &providers.GraphServiceClient{Client: client}, nil
return client, nil
}

func fillSetUpConfig(sc *providers.SetUpCmd) {
Expand Down
14 changes: 5 additions & 9 deletions pkg/providers/az-client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/subscription/armsubscription"

msgraph "github.com/microsoftgraph/msgraph-sdk-go"
graphapp "github.com/microsoftgraph/msgraph-sdk-go/applications"
)

type AzClient struct {
Expand All @@ -23,19 +24,14 @@ type azTenantClient interface {
NewListPager(options *armsubscription.TenantsClientListOptions) *runtime.Pager[armsubscription.TenantsClientListResponse]
}

// GraphServiceClient implements the GraphClient interface.
type GraphServiceClient struct {
Client *msgraph.GraphServiceClient
}

type GraphClient interface {
GetApplicationObjectId(ctx context.Context, appId string) (string, error)
Applications() *graphapp.ApplicationsRequestBuilder
Copy link
Member

Choose a reason for hiding this comment

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

I know we talked about creating an interface out of the ApplicationRequestBuilder so that we could mock the Post and other calls, are you still working through that or did you decide against it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried creating an interface, but the way is it used in other functions, I felt like it may not be a good idea. I can explain in more detail.

}

var _ GraphClient = &GraphServiceClient{}
var _ GraphClient = &msgraph.GraphServiceClient{}

func (g *GraphServiceClient) GetApplicationObjectId(ctx context.Context, appId string) (string, error) {
req := g.Client.Applications().ByApplicationId(appId)
func GetApplicationObjectId(ctx context.Context, appId string, graphClient GraphClient) (string, error) {
req := graphClient.Applications().ByApplicationId(appId)

app, err := req.Get(ctx, nil)
if err != nil {
Expand Down
38 changes: 14 additions & 24 deletions pkg/providers/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/subscription/armsubscription"
graphmodels "github.com/microsoftgraph/msgraph-sdk-go/models"
"os/exec"
"time"

Expand Down Expand Up @@ -46,7 +47,7 @@ func InitiateAzureOIDCFlow(ctx context.Context, sc *SetUpCmd, s spinner.Spinner)

if AzAppExists(sc.AppName) {
return errors.New("app already exists")
} else if err := sc.createAzApp(); err != nil {
} else if err := sc.createAzApp(ctx); err != nil {
return err
}

Expand Down Expand Up @@ -86,38 +87,27 @@ func InitiateAzureOIDCFlow(ctx context.Context, sc *SetUpCmd, s spinner.Spinner)
return nil
}

func (sc *SetUpCmd) createAzApp() error {
func (sc *SetUpCmd) createAzApp(ctx context.Context) error {
log.Debug("Commencing Azure app creation...")
start := time.Now()
log.Debug(start)

createApp := func() error {
createAppCmd := exec.Command("az", "ad", "app", "create", "--only-show-errors", "--display-name", sc.AppName)
requestBody := graphmodels.NewApplication()
displayName := sc.AppName
requestBody.SetDisplayName(&displayName)

out, err := createAppCmd.CombinedOutput()
application, err := sc.AzClient.GraphClient.Applications().Post(ctx, requestBody, nil)
if err != nil {
log.Printf("%s\n", out)
return err
return fmt.Errorf("creating Azure app: %v", err)
}
sc.appId = *application.GetId()

if AzAppExists(sc.AppName) {
var azApp map[string]interface{}
if err := json.Unmarshal(out, &azApp); err != nil {
return err
}
appId := fmt.Sprint(azApp["appId"])

sc.appId = appId

end := time.Since(start)
log.Debug("App created successfully!")
log.Debug(end)
return nil
}

return errors.New("app creation time has exceeded max elapsed time for exponential backoff")
end := time.Since(start)
log.Debug("App created successfully!")
log.Debug(end)
return nil
}

backoff := bo.NewExponentialBackOff()
backoff.MaxElapsedTime = 5 * time.Second

Expand Down Expand Up @@ -323,7 +313,7 @@ func (sc *SetUpCmd) createFederatedCredentials() error {
func (sc *SetUpCmd) getAppObjectId(ctx context.Context) error {
log.Debug("Fetching Azure application object ID")

appID, err := sc.AzClient.GraphClient.GetApplicationObjectId(ctx, sc.appId)
appID, err := GetApplicationObjectId(ctx, sc.appId, sc.AzClient.GraphClient)
if err != nil {
return fmt.Errorf("getting application object Id: %w", err)
}
Expand Down
Loading
Loading