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

Removed DBToken. Made the components uses Managed Identity #3584

Merged
merged 13 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ linters-settings:
alias: $1
- pkg: "^github\\.com/Azure/ARO-RP/pkg/client/services/redhatopenshift/mgmt/([0-9]+)-([0-9]+)-([0-9]+)-?(preview)?/redhatopenshift$"
alias: mgmtredhatopenshift$1$2$3$4
- pkg: "^github\\.com/Azure/ARO-RP/pkg/(dbtoken|deploy|gateway|mirror|monitor|operator|portal)$"
- pkg: "^github\\.com/Azure/ARO-RP/pkg/(deploy|gateway|mirror|monitor|operator|portal)$"
alias: pkg$1
- pkg: "^github\\.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/redhatopenshift/([0-9]+)-([0-9]+)-([0-9]+)-?(preview)?/redhatopenshift$"
alias: redhatopenshift$1$2$3$4
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.aro-multistage
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ FROM ${REGISTRY}/ubi8/ubi-minimal
RUN microdnf update && microdnf clean all
COPY --from=builder /app/aro /app/e2e.test /usr/local/bin/
ENTRYPOINT ["aro"]
EXPOSE 2222/tcp 8080/tcp 8443/tcp 8444/tcp 8445/tcp
EXPOSE 2222/tcp 8080/tcp 8443/tcp 8444/tcp
anshulvermapatel marked this conversation as resolved.
Show resolved Hide resolved
USER 1000
2 changes: 1 addition & 1 deletion Dockerfile.ci-rp
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,5 @@ LABEL aro-final=true
RUN microdnf update && microdnf clean all
COPY --from=builder /app/aro /app/e2e.test /usr/local/bin/
ENTRYPOINT ["aro"]
EXPOSE 2222/tcp 8080/tcp 8443/tcp 8444/tcp 8445/tcp
EXPOSE 2222/tcp 8080/tcp 8443/tcp 8444/tcp
USER 1000
1 change: 0 additions & 1 deletion cmd/aro/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const (
envDatabaseName = "DATABASE_NAME"
envDatabaseAccountName = "DATABASE_ACCOUNT_NAME"
envKeyVaultPrefix = "KEYVAULT_PREFIX"
envDBTokenUrl = "DBTOKEN_URL"
envOpenShiftVersions = "OPENSHIFT_VERSIONS"
envInstallerImageDigests = "INSTALLER_IMAGE_DIGESTS"
)
130 changes: 0 additions & 130 deletions cmd/aro/dbtoken.go

This file was deleted.

56 changes: 8 additions & 48 deletions cmd/aro/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ package main

import (
"context"
"fmt"
"os"
"os/signal"
"strings"
"syscall"
"time"

"github.com/sirupsen/logrus"

"github.com/Azure/ARO-RP/pkg/database"
pkgdbtoken "github.com/Azure/ARO-RP/pkg/dbtoken"
"github.com/Azure/ARO-RP/pkg/env"
pkggateway "github.com/Azure/ARO-RP/pkg/gateway"
"github.com/Azure/ARO-RP/pkg/metrics/statsd"
Expand All @@ -28,10 +26,6 @@ func gateway(ctx context.Context, log *logrus.Entry) error {
return err
}

if err = env.ValidateVars("AZURE_DBTOKEN_CLIENT_ID"); err != nil {
return err
}

m := statsd.New(ctx, log.WithField("component", "gateway"), _env, os.Getenv("MDM_ACCOUNT"), os.Getenv("MDM_NAMESPACE"), os.Getenv("MDM_STATSD_SOCKET"))

g, err := golang.NewMetrics(log.WithField("component", "gateway"), m)
Expand All @@ -44,36 +38,23 @@ func gateway(ctx context.Context, log *logrus.Entry) error {
if err := env.ValidateVars(envDatabaseAccountName); err != nil {
return err
}
dbc, err := database.NewDatabaseClient(log.WithField("component", "database"), _env, nil, m, nil, os.Getenv(envDatabaseAccountName))

msiToken, err := _env.NewMSITokenCredential()
if err != nil {
return err
}
logrusEntry := log.WithField("component", "database")

// Access token GET request needs to be:
// http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=$AZURE_DBTOKEN_CLIENT_ID
//
// In this context, the "resource" parameter is passed to azidentity as a
// "scope" argument even though a scope normally consists of an endpoint URL.
scope := os.Getenv("AZURE_DBTOKEN_CLIENT_ID")
msiRefresherAuthorizer, err := _env.NewMSIAuthorizer(scope)
dbAccountName := os.Getenv(envDatabaseAccountName)
scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
if err != nil {
return err
}

// TODO: refactor this poor man's feature flag
insecureSkipVerify := _env.IsLocalDevelopmentMode()
for _, feature := range strings.Split(os.Getenv("GATEWAY_FEATURES"), ",") {
if feature == "InsecureSkipVerifyDBTokenCertificate" {
insecureSkipVerify = true
break
}
}

url, err := getURL(_env.IsLocalDevelopmentMode())
dbc, err := database.NewDatabaseClient(logrusEntry, _env, dbAuthorizer, m, nil, dbAccountName)
if err != nil {
return err
}
dbRefresher := pkgdbtoken.NewRefresher(log, _env, msiRefresherAuthorizer, insecureSkipVerify, dbc, "gateway", m, "gateway", url)

dbName, err := DBName(_env.IsLocalDevelopmentMode())
if err != nil {
Expand All @@ -85,15 +66,6 @@ func gateway(ctx context.Context, log *logrus.Entry) error {
return err
}

go func() {
_ = dbRefresher.Run(ctx)
}()

log.Print("waiting for database token")
for !dbRefresher.HasSyncedOnce() {
time.Sleep(time.Second)
}

httpl, err := utilnet.Listen("tcp", ":8080", pkggateway.SocketSize)
if err != nil {
return err
Expand Down Expand Up @@ -130,15 +102,3 @@ func gateway(ctx context.Context, log *logrus.Entry) error {

return nil
}

func getURL(isLocalDevelopmentMode bool) (string, error) {
if isLocalDevelopmentMode {
return "https://localhost:8445", nil
}

if err := env.ValidateVars(envDBTokenUrl); err != nil {
return "", err
}

return os.Getenv(envDBTokenUrl), nil
}
4 changes: 0 additions & 4 deletions cmd/aro/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (

func usage() {
fmt.Fprint(flag.CommandLine.Output(), "usage:\n")
fmt.Fprintf(flag.CommandLine.Output(), " %s dbtoken\n", os.Args[0])
fmt.Fprintf(flag.CommandLine.Output(), " %s deploy config.yaml location\n", os.Args[0])
fmt.Fprintf(flag.CommandLine.Output(), " %s gateway\n", os.Args[0])
fmt.Fprintf(flag.CommandLine.Output(), " %s mirror [release_image...]\n", os.Args[0])
Expand Down Expand Up @@ -48,9 +47,6 @@ func main() {

var err error
switch strings.ToLower(flag.Arg(0)) {
case "dbtoken":
checkArgs(1)
err = dbtoken(ctx, log)
case "deploy":
checkArgs(3)
err = deploy(ctx, log)
Expand Down
10 changes: 5 additions & 5 deletions cmd/aro/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ package main

import (
"context"
"fmt"
"os"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy"
"github.com/Azure/go-autorest/tracing"
"github.com/sirupsen/logrus"
kmetrics "k8s.io/client-go/tools/metrics"
Expand Down Expand Up @@ -88,11 +88,10 @@ func monitor(ctx context.Context, log *logrus.Entry) error {
}

dbAccountName := os.Getenv(envDatabaseAccountName)
clientOptions := &policy.ClientOptions{
ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
}

logrusEntry := log.WithField("component", "database")
dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
if err != nil {
return err
}
Expand All @@ -106,6 +105,7 @@ func monitor(ctx context.Context, log *logrus.Entry) error {
if err != nil {
return err
}

dbMonitors, err := database.NewMonitors(ctx, dbc, dbName)
if err != nil {
return err
Expand Down
10 changes: 5 additions & 5 deletions cmd/aro/portal.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ package main
import (
"context"
"crypto/x509"
"fmt"
"net"
"os"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy"
"github.com/sirupsen/logrus"

"github.com/Azure/ARO-RP/pkg/database"
Expand Down Expand Up @@ -98,11 +98,10 @@ func portal(ctx context.Context, log *logrus.Entry, audit *logrus.Entry) error {
}

dbAccountName := os.Getenv(envDatabaseAccountName)
clientOptions := &policy.ClientOptions{
ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
}

logrusEntry := log.WithField("component", "database")
dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
if err != nil {
return err
}
Expand All @@ -116,6 +115,7 @@ func portal(ctx context.Context, log *logrus.Entry, audit *logrus.Entry) error {
if err != nil {
return err
}

dbOpenShiftClusters, err := database.NewOpenShiftClusters(ctx, dbc, dbName)
if err != nil {
return err
Expand Down
10 changes: 4 additions & 6 deletions cmd/aro/rp.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"os/signal"
"syscall"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy"
"github.com/Azure/go-autorest/tracing"
"github.com/sirupsen/logrus"
kmetrics "k8s.io/client-go/tools/metrics"
Expand Down Expand Up @@ -108,13 +107,11 @@ func rp(ctx context.Context, log, audit *logrus.Entry) error {
if err := env.ValidateVars(envDatabaseAccountName); err != nil {
return err
}

dbAccountName := os.Getenv(envDatabaseAccountName)
clientOptions := &policy.ClientOptions{
ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions,
}

logrusEntry := log.WithField("component", "database")
dbAuthorizer, err := database.NewMasterKeyAuthorizer(ctx, logrusEntry, msiToken, clientOptions, _env.SubscriptionID(), _env.ResourceGroup(), dbAccountName)
scope := []string{fmt.Sprintf("https://%s.%s", dbAccountName, _env.Environment().CosmosDBDNSSuffixScope)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I've seen this same code 3 times in this PR so far, would it be worth extracting into something well named?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, this was the way it was previously, I think lets address this in a separate PR

dbAuthorizer, err := database.NewTokenAuthorizer(ctx, logrusEntry, msiToken, dbAccountName, scope)
if err != nil {
return err
}
Expand All @@ -128,6 +125,7 @@ func rp(ctx context.Context, log, audit *logrus.Entry) error {
if err != nil {
return err
}

dbAsyncOperations, err := database.NewAsyncOperations(ctx, _env.IsLocalDevelopmentMode(), dbc, dbName)
if err != nil {
return err
Expand Down
Loading
Loading