Expose Dashboard URL only when provisioning operation succeeded (#2693)

* Extract dasboard url regex as const

* Assert that dashboard url is empty in the response to the provisioning request

* Replace const string literals duplicates with consts from broker pkg

* rm dashboard url from provisioning response on new provisioning op

* Add func to provide dashboard url

* Replace dashboard url value with ProvideURL func in broker responses

* Return dashboard url for provisioning request with own cluster plan

* Assert that GET instance response contains dashboard url

Author: szwedm

internal/broker/instance_create.go

@@ -15,15 +15,14 @@ import (
 	"slices"
 	"strings"
 
-	error2 "github.com/kyma-project/kyma-environment-broker/internal/error"
-
 	"github.com/kyma-project/kyma-environment-broker/common/gardener"
 	"github.com/kyma-project/kyma-environment-broker/common/hyperscaler/rules"
 	pkg "github.com/kyma-project/kyma-environment-broker/common/runtime"
 	"github.com/kyma-project/kyma-environment-broker/internal"
 	"github.com/kyma-project/kyma-environment-broker/internal/additionalproperties"
 	"github.com/kyma-project/kyma-environment-broker/internal/config"
 	"github.com/kyma-project/kyma-environment-broker/internal/dashboard"
+	error2 "github.com/kyma-project/kyma-environment-broker/internal/error"
 	"github.com/kyma-project/kyma-environment-broker/internal/euaccess"
 	"github.com/kyma-project/kyma-environment-broker/internal/hyperscalers/aws"
 	"github.com/kyma-project/kyma-environment-broker/internal/kubeconfig"
@@ -307,6 +306,10 @@ func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, de
 	logger.Info("Adding operation to provisioning queue")
 	b.queue.Add(operation.ID)
 
+	if !IsOwnClusterPlan(provisioningParameters.PlanID) {
+		dashboardURL = ""
+	}
+
 	return domain.ProvisionedServiceSpec{
 		IsAsync:       true,
 		OperationData: operation.ID,
@@ -831,7 +834,7 @@ func (b *ProvisionEndpoint) handleExistingOperation(operation *internal.Provisio
 	return domain.ProvisionedServiceSpec{
 		IsAsync:       true,
 		OperationData: operation.ID,
-		DashboardURL:  operation.DashboardURL,
+		DashboardURL:  dashboard.ProvideURL(instance, operation),
 		Metadata: domain.InstanceMetadata{
 			Labels: ResponseLabels(*instance, b.config.URL, b.kcBuilder),
 		},

internal/broker/instance_create_test.go

@@ -37,8 +37,8 @@ import (
 )
 
 const (
-	serviceID       = "47c9dcbf-ff30-448e-ab36-d3bad66ba281"
-	planID          = "4deee563-e5ec-4731-b9b1-53b42d855f0c"
+	serviceID       = broker.KymaServiceID
+	planID          = broker.AzurePlanID
 	clusterRegion   = "westeurope"
 	globalAccountID = "e8f7ec0a-0cd6-41f0-905d-5d1efa9fb6c4"
 	subAccountID    = "3cb65e5b-e455-4799-bf35-be46e8f5a533"
@@ -60,6 +60,7 @@ func TestProvision_Provision(t *testing.T) {
 	log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
 		Level: slog.LevelInfo,
 	}))
+
 	t.Run("new operation will be created", func(t *testing.T) {
 		// given
 		// #setup memory storage
@@ -115,7 +116,7 @@ func TestProvision_Provision(t *testing.T) {
 		require.NoError(t, err)
 		assert.Regexp(t, "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$", response.OperationData)
 		assert.NotEqual(t, instanceID, response.OperationData)
-		assert.Regexp(t, `^https:\/\/dashboard\.example\.com\/\?kubeconfigID=`, response.DashboardURL)
+		assert.Empty(t, response.DashboardURL)
 		assert.Equal(t, clusterName, response.Metadata.Labels["Name"])
 		assert.NotContains(t, response.Metadata.Labels, "APIServerURL")
 
@@ -134,7 +135,7 @@ func TestProvision_Provision(t *testing.T) {
 		require.NoError(t, err)
 
 		assert.Equal(t, instance.Parameters, operation.ProvisioningParameters)
-		assert.Regexp(t, `^https:\/\/dashboard\.example\.com\/\?kubeconfigID=`, response.DashboardURL)
+		assert.Empty(t, response.DashboardURL)
 		assert.Equal(t, instance.GlobalAccountID, globalAccountID)
 		assert.Equal(t, fixDNSProviders(), instance.InstanceDetails.ShootDNSProviders)
 		assert.Equal(t, pkg.Azure, instance.Provider)
@@ -411,7 +412,7 @@ func TestProvision_Provision(t *testing.T) {
 		// UUID with version 4 and variant 1 i.e RFC. 4122/DCE
 		assert.Regexp(t, "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$", response.OperationData)
 		assert.NotEqual(t, instanceID, response.OperationData)
-		assert.Regexp(t, `^https:\/\/dashboard\.example\.com\/\?kubeconfigID=`, response.DashboardURL)
+		assert.Empty(t, response.DashboardURL)
 		assert.Equal(t, clusterName, response.Metadata.Labels["Name"])
 		assert.NotContains(t, response.Metadata.Labels, "APIServerURL")
 
@@ -432,7 +433,7 @@ func TestProvision_Provision(t *testing.T) {
 		require.NoError(t, err)
 
 		assert.Equal(t, instance.Parameters, operation.ProvisioningParameters)
-		assert.Regexp(t, `^https:\/\/dashboard\.example\.com\/\?kubeconfigID=`, response.DashboardURL)
+		assert.Empty(t, response.DashboardURL)
 		assert.Equal(t, instance.GlobalAccountID, globalAccountID)
 		assert.Equal(t, fixDNSProviders(), instance.InstanceDetails.ShootDNSProviders)
 	})

internal/broker/instance_get.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 
 	"github.com/kyma-project/kyma-environment-broker/internal"
+	"github.com/kyma-project/kyma-environment-broker/internal/dashboard"
 
 	"github.com/kyma-project/kyma-environment-broker/internal/kubeconfig"
 	"github.com/kyma-project/kyma-environment-broker/internal/storage"
@@ -74,7 +75,7 @@ func (b *GetInstanceEndpoint) GetInstance(_ context.Context, instanceID string,
 	spec := domain.GetInstanceDetailsSpec{
 		ServiceID:    instance.ServiceID,
 		PlanID:       instance.ServicePlanID,
-		DashboardURL: instance.DashboardURL,
+		DashboardURL: dashboard.ProvideURL(instance, op),
 		Parameters:   parameters,
 		Metadata: domain.InstanceMetadata{
 			Labels: ResponseLabels(*instance, b.config.URL, b.kcBuilder),

internal/broker/instance_get_test.go

@@ -103,6 +103,7 @@ func TestGetEndpoint_GetProvisioningInstance(t *testing.T) {
 	response, err := getSvc.GetInstance(context.Background(), instanceID, domain.FetchInstanceDetails{})
 	assert.Equal(t, nil, err, "Get returned error when expected to pass")
 	assert.Len(t, response.Metadata.Labels, 1)
+	assert.Regexp(t, dashboardUrlRegex, response.DashboardURL)
 }
 
 func TestGetEndpoint_DoNotReturnInstanceWhereDeletedAtIsNotZero(t *testing.T) {

internal/broker/instance_update.go

@@ -212,7 +212,7 @@ func (b *UpdateEndpoint) Update(ctx context.Context, instanceID string, details
 	}
 	return domain.UpdateServiceSpec{
 		IsAsync:       false,
-		DashboardURL:  dashboardURL,
+		DashboardURL:  dashboard.ProvideURL(instance, lastProvisioningOperation),
 		OperationData: "",
 		Metadata: domain.InstanceMetadata{
 			Labels: ResponseLabels(*instance, b.config.URL, b.kcBuilder),
@@ -252,7 +252,7 @@ func (b *UpdateEndpoint) processUpdateParameters(ctx context.Context, instance *
 		logger.Debug("Parameters not provided, skipping processing update parameters")
 		return domain.UpdateServiceSpec{
 			IsAsync:       false,
-			DashboardURL:  instance.DashboardURL,
+			DashboardURL:  dashboard.ProvideURL(instance, lastProvisioningOperation),
 			OperationData: "",
 			Metadata: domain.InstanceMetadata{
 				Labels: ResponseLabels(*instance, b.config.URL, b.kcBuilder),
@@ -517,7 +517,7 @@ func (b *UpdateEndpoint) processUpdateParameters(ctx context.Context, instance *
 
 	return domain.UpdateServiceSpec{
 		IsAsync:       true,
-		DashboardURL:  instance.DashboardURL,
+		DashboardURL:  dashboard.ProvideURL(instance, lastProvisioningOperation),
 		OperationData: operation.ID,
 		Metadata: domain.InstanceMetadata{
 			Labels: ResponseLabels(*instance, b.config.URL, b.kcBuilder),

internal/broker/instance_update_test.go

@@ -41,6 +41,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+const dashboardUrlRegex = `^https:\/\/dashboard\.example\.com\/\?kubeconfigID=`
+
 var dashboardConfig = dashboard.Config{LandscapeURL: "https://dashboard.example.com"}
 var fakeKcpK8sClient = fake.NewClientBuilder().Build()
 var imConfigFixture = broker.InfrastructureManager{
@@ -1102,9 +1104,9 @@ func TestUpdateEndpoint_UpdateWithEnabledDashboard(t *testing.T) {
 	require.NoError(t, err)
 
 	// check if the instance is updated successfully
-	assert.Regexp(t, `^https:\/\/dashboard\.example\.com\/\?kubeconfigID=`, inst.DashboardURL)
+	assert.Regexp(t, dashboardUrlRegex, inst.DashboardURL)
 	// check if the API response is correct
-	assert.Regexp(t, `^https:\/\/dashboard\.example\.com\/\?kubeconfigID=`, response.DashboardURL)
+	assert.Regexp(t, dashboardUrlRegex, response.DashboardURL)
 }
 
 func TestUpdateExpiredInstance(t *testing.T) {

internal/dashboard/url_provider.go

@@ -0,0 +1,13 @@
+package dashboard
+
+import (
+	"github.com/kyma-project/kyma-environment-broker/internal"
+	"github.com/pivotal-cf/brokerapi/v12/domain"
+)
+
+func ProvideURL(instance *internal.Instance, provisioningOperation *internal.ProvisioningOperation) string {
+	if provisioningOperation.State == domain.Succeeded {
+		return instance.DashboardURL
+	}
+	return ""
+}