changed the structure of fetchers and test cases.

pkg/portal/cluster.go

@@ -185,13 +185,13 @@ func (p *portal) machines(w http.ResponseWriter, r *http.Request) {
 func (p *portal) VMAllocationStatus(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
-	fetcher, err := p.makeFetcher(ctx, r)
+	azurefetcher, err := p.makeAzureFetcher(ctx, r)
 	if err != nil {
 		p.internalServerError(w, err)
 		return
 	}
 
-	machineVMAllocationStatus, err := fetcher.VMAllocationStatus(ctx)
+	machineVMAllocationStatus, err := azurefetcher.VMAllocationStatus(ctx)
 	if err != nil {
 		p.internalServerError(w, err)
 		return

pkg/portal/cluster/fetcher.go

@@ -34,12 +34,15 @@ type VirtualMachinesClientFactory interface {
 // information about clusters. It returns frontend-suitable data structures.
 type FetchClient interface {
 	Nodes(context.Context) (*NodeListInformation, error)
-	VMAllocationStatus(context.Context) (map[string]string, error)
 	ClusterOperators(context.Context) (*ClusterOperatorsInformation, error)
 	Machines(context.Context) (*MachineListInformation, error)
 	MachineSets(context.Context) (*MachineSetListInformation, error)
 }
 
+type AzureFetchClient interface {
+	VMAllocationStatus(context.Context) (map[string]string, error)
+}
+
 // client is an implementation of FetchClient. It currently contains a "fetcher"
 // which is responsible for fetching information from the k8s clusters. The
 // mechanism of fetching the data from the cluster and returning it to the
@@ -56,19 +59,28 @@ type client struct {
 // contains Kubernetes clients and returns the frontend-suitable data
 // structures. The concrete implementation of FetchClient wraps this.
 type realFetcher struct {
-	log                          *logrus.Entry
+	log           *logrus.Entry
-	configCli                    configclient.Interface
+	configCli     configclient.Interface
-	kubernetesCli                kubernetes.Interface
+	kubernetesCli kubernetes.Interface
-	machineClient                machineclient.Interface
+	machineClient machineclient.Interface
-	azureSideFetcher             azureSideFetcher
-	resourceClientFactory        ResourceClientFactory
-	virtualMachinesClientFactory VirtualMachinesClientFactory
 }
 
+// azureClient is the same implementation as client's, the only difference is that it will be used to fetch something from azure regarding a cluster.
+type azureClient struct {
+	log     *logrus.Entry
+	fetcher *azureSideFetcher
+}
+
+// azureSideFetcher is responsible for fetching information about azure resources of a k8s cluster. It
+// contains azure related authentication/authorization data and returns the frontend-suitable data
+// structures. The concrete implementation of AzureFetchClient wraps this.
 type azureSideFetcher struct {
-	resourceGroupName string
+	log                          *logrus.Entry
-	subscriptionDoc   *api.SubscriptionDocument
+	resourceGroupName            string
-	env               env.Interface
+	subscriptionDoc              *api.SubscriptionDocument
+	env                          env.Interface
+	resourceClientFactory        ResourceClientFactory
+	virtualMachinesClientFactory VirtualMachinesClientFactory
 }
 
 type clientFactory struct{}
@@ -81,15 +93,18 @@ func (cf clientFactory) NewVirtualMachinesClient(environment *azureclient.AROEnv
 	return compute.NewVirtualMachinesClient(environment, subscriptionID, authorizer)
 }
 
-func newAzureSideFetcher(resourceGroupName string, subscriptionDoc *api.SubscriptionDocument, env env.Interface) azureSideFetcher {
+func newAzureSideFetcher(log *logrus.Entry, resourceGroupName string, subscriptionDoc *api.SubscriptionDocument, env env.Interface, resourceClientFactory ResourceClientFactory, virtualMachinesClientFactory VirtualMachinesClientFactory) azureSideFetcher {
 	return azureSideFetcher{
-		resourceGroupName: resourceGroupName,
+		log:                          log,
-		subscriptionDoc:   subscriptionDoc,
+		resourceGroupName:            resourceGroupName,
-		env:               env,
+		subscriptionDoc:              subscriptionDoc,
+		env:                          env,
+		resourceClientFactory:        resourceClientFactory,
+		virtualMachinesClientFactory: virtualMachinesClientFactory,
 	}
 }
 
-func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftClusterDocument, azureSideFetcher azureSideFetcher, resourceClientFactory ResourceClientFactory, virtualMachinesClientFactory VirtualMachinesClientFactory) (*realFetcher, error) {
+func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftClusterDocument) (*realFetcher, error) {
 	restConfig, err := restconfig.RestConfig(dialer, doc.OpenShiftCluster)
 	if err != nil {
 		log.Error(err)
@@ -114,21 +129,15 @@ func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftCl
 	}
 
 	return &realFetcher{
-		log:                          log,
+		log:           log,
-		configCli:                    configCli,
+		configCli:     configCli,
-		kubernetesCli:                kubernetesCli,
+		kubernetesCli: kubernetesCli,
-		machineClient:                machineClient,
+		machineClient: machineClient,
-		azureSideFetcher:             azureSideFetcher,
-		resourceClientFactory:        resourceClientFactory,
-		virtualMachinesClientFactory: virtualMachinesClientFactory,
 	}, nil
 }
 
-func NewFetchClient(log *logrus.Entry, dialer proxy.Dialer, cluster *api.OpenShiftClusterDocument, subscriptionDoc *api.SubscriptionDocument, env env.Interface) (FetchClient, error) {
+func NewFetchClient(log *logrus.Entry, dialer proxy.Dialer, cluster *api.OpenShiftClusterDocument) (FetchClient, error) {
-	resourceGroupName := stringutils.LastTokenByte(cluster.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/')
+	fetcher, err := newRealFetcher(log, dialer, cluster)
-	azureSideFetcher := newAzureSideFetcher(resourceGroupName, subscriptionDoc, env)
-	cf := clientFactory{}
-	fetcher, err := newRealFetcher(log, dialer, cluster, azureSideFetcher, cf, cf)
 	if err != nil {
 		return nil, err
 	}
@@ -139,3 +148,13 @@ func NewFetchClient(log *logrus.Entry, dialer proxy.Dialer, cluster *api.OpenShi
 		fetcher: fetcher,
 	}, nil
 }
+
+func NewAzureFetchClient(log *logrus.Entry, cluster *api.OpenShiftClusterDocument, subscriptionDoc *api.SubscriptionDocument, env env.Interface) AzureFetchClient {
+	resourceGroupName := stringutils.LastTokenByte(cluster.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/')
+	cf := clientFactory{}
+	azureSideFetcher := newAzureSideFetcher(log, resourceGroupName, subscriptionDoc, env, cf, cf)
+	return &azureClient{
+		log:     log,
+		fetcher: &azureSideFetcher,
+	}
+}

pkg/portal/cluster/machines.go

@@ -65,25 +65,26 @@ func (c *client) Machines(ctx context.Context) (*MachineListInformation, error)
 	return c.fetcher.Machines(ctx)
 }
 
-func (c *client) VMAllocationStatus(ctx context.Context) (map[string]string, error) {
+func (c *azureClient) VMAllocationStatus(ctx context.Context) (map[string]string, error) {
 	return c.fetcher.vmAllocationStatus(ctx)
 }
 
-func (f *realFetcher) vmAllocationStatus(ctx context.Context) (map[string]string, error) {
+func (f *azureSideFetcher) vmAllocationStatus(ctx context.Context) (map[string]string, error) {
-	env := f.azureSideFetcher.env
+	env := f.env
-	subscriptionDoc := f.azureSideFetcher.subscriptionDoc
+	subscriptionDoc := f.subscriptionDoc
-	clusterRGName := f.azureSideFetcher.resourceGroupName
+	clusterRGName := f.resourceGroupName
-	fpAuth, err := env.FPAuthorizer(subscriptionDoc.Subscription.Properties.TenantID, env.Environment().ResourceManagerEndpoint)
+	aroEnvironment := env.Environment()
+	fpAuth, err := env.FPAuthorizer(subscriptionDoc.Subscription.Properties.TenantID, aroEnvironment.ResourceManagerEndpoint)
 	if err != nil {
 		return nil, err
 	}
 	// Getting Virtual Machine resources through the Cluster's Resource Group
-	computeResources, err := f.resourceClientFactory.NewResourcesClient(env.Environment(), subscriptionDoc.ID, fpAuth).ListByResourceGroup(ctx, clusterRGName, "resourceType eq 'Microsoft.Compute/virtualMachines'", "", nil)
+	computeResources, err := f.resourceClientFactory.NewResourcesClient(aroEnvironment, subscriptionDoc.ID, fpAuth).ListByResourceGroup(ctx, clusterRGName, "resourceType eq 'Microsoft.Compute/virtualMachines'", "", nil)
 	if err != nil {
 		return nil, err
 	}
 	vmAllocationStatus := make(map[string]string)
-	virtualMachineClient := f.virtualMachinesClientFactory.NewVirtualMachinesClient(env.Environment(), subscriptionDoc.ID, fpAuth)
+	virtualMachineClient := f.virtualMachinesClientFactory.NewVirtualMachinesClient(aroEnvironment, subscriptionDoc.ID, fpAuth)
 	for _, res := range computeResources {
 		putAllocationStatusToMap(ctx, clusterRGName, vmAllocationStatus, res, virtualMachineClient, f.log)
 	}
@@ -107,7 +108,6 @@ func putAllocationStatusToMap(ctx context.Context, clusterRGName string, vmAlloc
 			return
 		}
 	}
-
 	vmAllocationStatus[vmName] = ""
 }
 

pkg/portal/cluster/machines_test.go

@@ -132,88 +132,36 @@ func (mcf MockClientFactory) NewVirtualMachinesClient(environment *azureclient.A
 func TestVMAllocationStatus(t *testing.T) {
 	ctx := context.Background()
 	controller := gomock.NewController(t)
-	mockResourcesClient := mock_features.NewMockResourcesClient(controller)
-	mockVirtualMachinesClient := mock_compute.NewMockVirtualMachinesClient(controller)
 	type test struct {
 		name    string
-		mocks   func(*test, *mock_env.MockInterface, *mock_refreshable.MockAuthorizer, *mock_features.MockResourcesClient, *mock_compute.MockVirtualMachinesClient)
+		mocks   map[string]int
 		wantErr string
 	}
 	for _, tt := range []*test{
 		{
 			name: "Successfully fetching VMs allocation status. Calling all the required methods.",
-			mocks: func(tt *test,
+			mocks: map[string]int{
-				env *mock_env.MockInterface,
+				"env.Environment":                    1,
-				authorizer *mock_refreshable.MockAuthorizer,
+				"env.FPAuthorizer":                   1,
-				mockResourcesClient *mock_features.MockResourcesClient,
+				"resourceClient.ListByResourceGroup": 1,
-				mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient) {
+				"virtualMachinesClient.Get":          1,
-				env.EXPECT().Environment().Return(&azureclient.AROEnvironment{
-					Environment: azure.Environment{
-						ResourceManagerEndpoint: "temp",
-					},
-				}).AnyTimes()
-				env.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(authorizer, nil)
-
-				mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
-					Return([]mgmtfeatures.GenericResourceExpanded{
-						{
-							Kind: func(v string) *string { return &v }("something"),
-							Type: func(v string) *string { return &v }("Microsoft.Compute/virtualMachines"),
-							Name: func(v string) *string { return &v }("master-x"),
-						},
-					}, nil)
-
-				mockVirtualMachinesClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mgmtcompute.VirtualMachine{
-					Name: func(v string) *string { return &v }("master-x"),
-					VirtualMachineProperties: &mgmtcompute.VirtualMachineProperties{
-						InstanceView: &mgmtcompute.VirtualMachineInstanceView{
-							Statuses: &[]mgmtcompute.InstanceViewStatus{
-								{
-									Code: func() *string {
-										s := new(string)
-										*s = "PowerState/running"
-										return s
-									}(),
-								},
-							},
-						},
-					},
-				}, nil)
 			},
 			wantErr: "",
 		},
 		{
 			name: "No VM resource found",
-			mocks: func(tt *test,
+			mocks: map[string]int{
-				env *mock_env.MockInterface,
+				"env.Environment":                    1,
-				authorizer *mock_refreshable.MockAuthorizer,
+				"env.FPAuthorizer":                   1,
-				mockResourcesClient *mock_features.MockResourcesClient,
+				"resourceClient.ListByResourceGroup": 1,
-				mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient) {
-				env.EXPECT().Environment().Return(&azureclient.AROEnvironment{
-					Environment: azure.Environment{
-						ResourceManagerEndpoint: "temp",
-					},
-				}).AnyTimes()
-				env.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(authorizer, nil)
-
-				mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
-					Return([]mgmtfeatures.GenericResourceExpanded{}, nil)
 			},
 			wantErr: "",
 		},
 		{
 			name: "Empty FP Authorizer",
-			mocks: func(tt *test,
+			mocks: map[string]int{
-				env *mock_env.MockInterface,
+				"env.Environment":  1,
-				authorizer *mock_refreshable.MockAuthorizer,
+				"env.FPAuthorizer": 1,
-				mockResourcesClient *mock_features.MockResourcesClient,
-				mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient) {
-				env.EXPECT().Environment().Return(&azureclient.AROEnvironment{
-					Environment: azure.Environment{
-						ResourceManagerEndpoint: "temp",
-					},
-				}).AnyTimes()
-				env.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(nil, errors.New("Empty Athorizer"))
 			},
 			wantErr: "Empty Athorizer",
 		},
@@ -221,6 +169,8 @@ func TestVMAllocationStatus(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			mockEnv := mock_env.NewMockInterface(controller)
 			mockRefreshable := mock_refreshable.NewMockAuthorizer(controller)
+			mockResourcesClient := mock_features.NewMockResourcesClient(controller)
+			mockVirtualMachinesClient := mock_compute.NewMockVirtualMachinesClient(controller)
 			subscriptionDoc := &api.SubscriptionDocument{
 				ID:          "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx",
 				ResourceID:  "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx",
@@ -240,23 +190,77 @@ func TestVMAllocationStatus(t *testing.T) {
 			}
 
 			if tt.mocks != nil {
-				tt.mocks(tt, mockEnv, mockRefreshable, mockResourcesClient, mockVirtualMachinesClient)
+				if tt.mocks["env.Environment"] > 0 {
+					switch tt.name {
+					case "Successfully fetching VMs allocation status. Calling all the required methods.", "No VM resource found", "Empty FP Authorizer":
+						mockEnv.EXPECT().Environment().Return(&azureclient.AROEnvironment{
+							Environment: azure.Environment{
+								ResourceManagerEndpoint: "temp",
+							},
+						})
+					}
+				}
+
+				if tt.mocks["env.FPAuthorizer"] > 0 {
+					switch tt.name {
+					case "Successfully fetching VMs allocation status. Calling all the required methods.", "No VM resource found":
+						mockEnv.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(mockRefreshable, nil)
+
+					case "Empty FP Authorizer":
+						mockEnv.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(nil, errors.New("Empty Athorizer"))
+					}
+				}
+
+				if tt.mocks["resourceClient.ListByResourceGroup"] > 0 {
+					switch tt.name {
+					case "Successfully fetching VMs allocation status. Calling all the required methods.":
+						mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+							Return([]mgmtfeatures.GenericResourceExpanded{
+								{
+									Kind: func(v string) *string { return &v }("something"),
+									Type: func(v string) *string { return &v }("Microsoft.Compute/virtualMachines"),
+									Name: func(v string) *string { return &v }("master-x"),
+								},
+							}, nil)
+					case "No VM resource found":
+						mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+							Return([]mgmtfeatures.GenericResourceExpanded{}, nil)
+					}
+				}
+				if tt.mocks["virtualMachinesClient.Get"] > 0 {
+					switch tt.name {
+					case "Successfully fetching VMs allocation status. Calling all the required methods.":
+						mockVirtualMachinesClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mgmtcompute.VirtualMachine{
+							Name: func(v string) *string { return &v }("master-x"),
+							VirtualMachineProperties: &mgmtcompute.VirtualMachineProperties{
+								InstanceView: &mgmtcompute.VirtualMachineInstanceView{
+									Statuses: &[]mgmtcompute.InstanceViewStatus{
+										{
+											Code: func() *string {
+												s := new(string)
+												*s = "PowerState/running"
+												return s
+											}(),
+										},
+									},
+								},
+							},
+						}, nil)
+					}
+				}
 			}
 			_, log := testlog.New()
-			azureSideFetcher := azureSideFetcher{
-				resourceGroupName: "someResourceGroup",
-				env:               mockEnv,
-				subscriptionDoc:   subscriptionDoc,
-			}
 			mcf := newMockClientFactory(mockResourcesClient, mockVirtualMachinesClient)
-			realFetcher := &realFetcher{
+			azureSideFetcher := &azureSideFetcher{
 				log:                          log,
-				azureSideFetcher:             azureSideFetcher,
+				resourceGroupName:            "someResourceGroup",
+				subscriptionDoc:              subscriptionDoc,
+				env:                          mockEnv,
 				resourceClientFactory:        mcf,
 				virtualMachinesClientFactory: mcf,
 			}
-			client := &client{fetcher: realFetcher, log: log}
+			azureClient := &azureClient{fetcher: azureSideFetcher, log: log}
-			_, err := client.VMAllocationStatus(ctx)
+			_, err := azureClient.VMAllocationStatus(ctx)
 			if err != nil && err.Error() != tt.wantErr || err == nil && tt.wantErr != "" {
 				t.Error("Expected", tt.wantErr, "Got", err)
 			}

pkg/portal/portal.go

@@ -47,16 +47,13 @@ type Runnable interface {
 }
 
 type portal struct {
-	env                 env.Interface
+	env           env.Interface
-	audit               *logrus.Entry
+	audit         *logrus.Entry
-	log                 *logrus.Entry
+	log           *logrus.Entry
-	baseAccessLog       *logrus.Entry
+	baseAccessLog *logrus.Entry
-	l                   net.Listener
+	l             net.Listener
-	sshl                net.Listener
+	sshl          net.Listener
-	verifier            oidc.Verifier
+	verifier      oidc.Verifier
-	baseRouter          *mux.Router
-	authenticatedRouter *mux.Router
-	publicRouter        *mux.Router
 
 	hostname     string
 	servingKey   *rsa.PrivateKey
@@ -400,13 +397,37 @@ func (p *portal) makeFetcher(ctx context.Context, r *http.Request) (cluster.Fetc
 	} else {
 		dialer = p.dialer
 	}
+	return cluster.NewFetchClient(p.log, dialer, doc)
+}
+
+// makeAzureFetcher creates a cluster.AzureFetchClient suitable for use by the Portal REST API to fetch anything directly from Azure like VM Details etc.
+func (p *portal) makeAzureFetcher(ctx context.Context, r *http.Request) (cluster.AzureFetchClient, error) {
+	apiVars := mux.Vars(r)
+
+	subscription := apiVars["subscription"]
+	resourceGroup := apiVars["resourceGroup"]
+	clusterName := apiVars["clusterName"]
+
+	resourceID :=
+		strings.ToLower(
+			fmt.Sprintf(
+				"/subscriptions/%s/resourceGroups/%s/providers/Microsoft.RedHatOpenShift/openShiftClusters/%s",
+				subscription, resourceGroup, clusterName))
+	if !validate.RxClusterID.MatchString(resourceID) {
+		return nil, fmt.Errorf("invalid resource ID")
+	}
+
+	doc, err := p.dbOpenShiftClusters.Get(ctx, resourceID)
+	if err != nil {
+		return nil, err
+	}
 
 	subscriptionDoc, err := p.getSubscriptionDocument(ctx, doc.Key)
 	if err != nil {
 		return nil, err
 	}
 
-	return cluster.NewFetchClient(p.log, dialer, doc, subscriptionDoc, p.env)
+	return cluster.NewAzureFetchClient(p.log, doc, subscriptionDoc, p.env), nil
 }
 
 func (p *portal) serve(path string) func(w http.ResponseWriter, r *http.Request) {