documentation

docs/feature-flags.md

@@ -0,0 +1,47 @@
+# Feature flags
+
+Feature flags are used in a couple of different places in the ARO-RP codebase.
+
+## Subscription feature flags
+
+Azure has the capability to set feature flags on subscriptions.  Depending on
+the feature flag, it may be settable directly via the end user, or solely via
+Geneva.  See the ARM wiki for more details.
+
+ARM advertises subscription flags to RPs as part of the [subscription
+lifecycle](https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/subscription-lifecycle-api-reference.md);
+the ARO RP responds to subscription PUTs by storing the subscription document
+verbatim in Cosmos DB.  Thus subscription feature flags can be checked by
+reading the subscription document from the database.
+
+Subscription feature flags used by ARO-RP include:
+
+* Microsoft.RedHatOpenShift/RedHatEngineering: generic feature flag for
+  graduating features to production such that they only apply on our Red Hat
+  engineering subscription.
+
+* Microsoft.RedHatOpenShift/SaveAROTestConfig: used on our test subscriptions to
+  cause the pkg/billing to copy billing data to a storage account so that it can
+  be validated by the billing E2E tests.
+
+Subscription feature flags are also used for API preview, INT and region
+rollout. See the RP ARM manifest for more details.
+
+## RP feature flags
+
+The RP_FEATURES environment variable is a comma-delimited list of RP codebase
+feature flags defined in pkg/env/env.go.  At the time of writing these include:
+
+* DisableDenyAssignments: don't create a deny assignment on the cluster resource
+  group.  Used everywhere except PROD, as this capability is not released
+  outside of PROD.
+
+* DisableSignedCertificates: don't integrate with Digicert to sign cluster
+  certificates.  Used in development only.
+
+* EnableDevelopmentAuthorizer: use the SubjectNameAndIssuer authorizer instead
+  of the ARM authorizer to validate inbound ARM API calls.  Used in development
+  only.
+
+* RequireD2sV3Workers: require cluster worker VMs to be Standard_D2s_v3 SKU.
+  Used in development only (to save money :-).

pkg/env/armhelper.go

@@ -22,16 +22,43 @@ import (
 	"github.com/Azure/ARO-RP/pkg/util/refreshable"
 )
 
+// In INT or PROD, when the ARO RP is running behind ARM, ARM follows the RP's
+// manifest configuration to grant the RP's first party service principal
+// limited standing access into customers' subscriptions (when the RP is
+// registered in the customer's AAD tenant).
+//
+// Our ARM manifest is set up such that the RP first party service principal can
+// create new resource groups in any customer subscription (when the RP is
+// registered in the customer's AAD tenant).  ARM invisibly then grants the RP
+// Owner access on the created resource group.  For more information, read the
+// ARM wiki ("ResourceGroup Scoped Service to Service Authorization") and the RP
+// manifest.
+//
+// When running outside INT or PROD (i.e. in development or in CI), ARMHelper is
+// used to fake up the above functionality with a separate development service
+// principal (AZURE_ARM_CLIENT_ID).  We use a separate SP so that the RP's
+// permissions are identical in dev and prod.  The advantage is that this helps
+// prevent a developer rely on a permission in dev only to find that permission
+// doesn't exist in prod.  The disadvantage is that an additional SP and this
+// helper code is required in dev.
+//
+// There remains one other minor difference between running in dev and INT/PROD:
+// in the former, the Owner role assignment created by the helper is visible.
+// In INT/PROD I believe it is invisible.
+
 type ARMHelper interface {
 	EnsureARMResourceGroupRoleAssignment(context.Context, refreshable.Authorizer, string) error
 }
 
+// noopARMHelper is used in INT and PROD.  It does nothing.
 type noopARMHelper struct{}
 
 func (*noopARMHelper) EnsureARMResourceGroupRoleAssignment(context.Context, refreshable.Authorizer, string) error {
 	return nil
 }
 
+// armHelper is used in dev.  It adds an Owner role assignment for the RP,
+// faking up what ARM would do invisibly for us in INT/PROD.
 type armHelper struct {
 	log *logrus.Entry
 	env Interface

pkg/env/core.go

@@ -14,6 +14,9 @@ import (
 	"github.com/Azure/ARO-RP/pkg/util/rpauthorizer"
 )
 
+// Core collects basic configuration information which is expected to be
+// available on any PROD service VMSS (i.e. instance metadata, MSI authorizer,
+// etc.)
 type Core interface {
 	IsLocalDevelopmentMode() bool
 	instancemetadata.InstanceMetadata

pkg/env/env.go

@@ -49,6 +49,14 @@ const (
 	RPPrivateEndpointPrefix          = "rp-pe-"
 )
 
+// Interface is clunky and somewhat legacy and only used in the RP codebase (not
+// monitor/portal/gateway, etc.).  It is a grab-bag of items which modify RP
+// behaviour depending on where it is running (dev, prod, etc.)  Outside of the
+// RP codebase, use Core.  Ideally we might break Interface into smaller pieces,
+// either closer to their point of use, or maybe using dependency injection. Try
+// to remove methods, not add more.  A refactored approach to configuration is
+// generally necessary across all of the ARO services; dealing with Interface
+// should be part of that.
 type Interface interface {
 	Core
 	proxy.Dialer

pkg/proxy/dialer.go

@@ -108,6 +108,11 @@ func (d *dev) DialContext(ctx context.Context, network, address string) (net.Con
 	return &conn{Conn: c, r: r}, nil
 }
 
+// NewDialer returns a Dialer which can dial a customer cluster API server. When
+// not in local development mode, there is no magic here.  In local development
+// mode, this dials the development proxy, which proxies to the requested
+// endpoint.  This enables the RP to run without routeability to its vnet in
+// local development mode.
 func NewDialer(isLocalDevelopmentMode bool) (Dialer, error) {
 	if !isLocalDevelopmentMode {
 		return &prod{}, nil

pkg/proxy/proxy.go

@@ -111,6 +111,10 @@ func (s *Server) Run() error {
 	}))
 }
 
+// proxy takes an HTTP/1.x CONNECT Request and ResponseWriter from the Golang
+// HTTP stack and uses Hijack() to get the underlying Connection (c1).  It dials
+// a second Connection (c2) to the requested end Host and then copies data in
+// both directions (c1->c2 and c2->c1).
 func proxy(log *logrus.Entry, w http.ResponseWriter, r *http.Request) {
 	c2, err := net.Dial("tcp", r.Host)
 	if err != nil {
@@ -126,6 +130,10 @@ func proxy(log *logrus.Entry, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Do as much setup as possible before calling Hijack(), because after
+	// Hijack() is called we have no mechanism to report errors back to the
+	// caller.
+
 	w.WriteHeader(http.StatusOK)
 
 	c1, buf, err := hijacker.Hijack()
@@ -138,6 +146,7 @@ func proxy(log *logrus.Entry, w http.ResponseWriter, r *http.Request) {
 	ch := make(chan struct{})
 
 	go func() {
+		// use a goroutine to copy from c1->c2.  Call c2.CloseWrite() when done.
 		defer recover.Panic(log)
 		defer close(ch)
 		defer func() {
@@ -147,11 +156,14 @@ func proxy(log *logrus.Entry, w http.ResponseWriter, r *http.Request) {
 	}()
 
 	func() {
+		// copy from c2->c1.  Call c1.CloseWrite() when done.
 		defer func() {
 			_ = c1.(*tls.Conn).CloseWrite()
 		}()
 		_, _ = io.Copy(c1, c2)
 	}()
 
+	// wait for the c1->c2 goroutine to complete.  Then the deferred c1.Close()
+	// and c2.Close() will be called.
 	<-ch
 }