diff --git a/cmd/broker/main.go b/cmd/broker/main.go index ddf54709c6..40083088f5 100644 --- a/cmd/broker/main.go +++ b/cmd/broker/main.go @@ -18,7 +18,6 @@ import ( "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/appinfo" "github.com/kyma-project/kyma-environment-broker/internal/broker" brokerBindings "github.com/kyma-project/kyma-environment-broker/internal/broker/bindings" @@ -240,11 +239,6 @@ func main() { fatalOnError(err, log) skrK8sClientProvider := kubeconfig.NewK8sClientFromSecretProvider(kcpK8sClient) - if cfg.Broker.MonitorAdditionalProperties { - err := os.MkdirAll(cfg.Broker.AdditionalPropertiesPath, os.ModePerm) - fatalOnError(err, log) - } - // create storage cipher := storage.NewEncrypter(cfg.Database.SecretKey) var db storage.BrokerStorage @@ -374,10 +368,6 @@ func main() { log) runtimeHandler.AttachRoutes(router) - // create list requests with additional properties endpoint - additionalPropertiesHandler := additionalproperties.NewHandler(log, cfg.Broker.AdditionalPropertiesPath) - additionalPropertiesHandler.AttachRoutes(router) - // create expiration endpoint expirationHandler := expiration.NewHandler(db.Instances(), db.Operations(), deprovisionQueue, log) expirationHandler.AttachRoutes(router) diff --git a/docs/contributor/02-30-keb-configuration.md b/docs/contributor/02-30-keb-configuration.md index 2280318176..5e04c84288 100644 --- a/docs/contributor/02-30-keb-configuration.md +++ b/docs/contributor/02-30-keb-configuration.md @@ -22,7 +22,6 @@ Kyma Environment Broker (KEB) binary allows you to override some configuration p | **APP_BROKER_FREE_​EXPIRATION_PERIOD** | 720h | Determines when to show expiration info to users | | **APP_BROKER_GARDENER_​SEEDS_CACHE_CONFIG_​MAP_NAME** | None | - | | **APP_BROKER_INCLUDE_​ADDITIONAL_PARAMS_​IN_SCHEMA** | false | If true, additional (advanced or less common) parameters are included in the provisioning schema for service plans | -| **APP_BROKER_MONITOR_​ADDITIONAL_​PROPERTIES** | false | If true, collects properties from the provisioning request that are not explicitly defined in the schema and stores them in persistent storage | | **APP_BROKER_ONLY_ONE_​FREE_PER_GA** | false | If true, restricts each global account to only one free (freemium) Kyma runtime. When enabled, provisioning another free environment for the same global account is blocked even if the previous one is deprovisioned | | **APP_BROKER_ONLY_​SINGLE_TRIAL_PER_GA** | true | If true, restricts each global account to only one active trial Kyma runtime at a time When enabled, provisioning another trial environment for the same global account is blocked until the previous one is deprovisioned | | **APP_BROKER_​OPERATION_TIMEOUT** | 7h | Maximum allowed duration for processing a single operation (provisioning, deprovisioning, etc.) If the operation exceeds this timeout, it is marked as failed. Example: "7h" for 7 hours | diff --git a/internal/additionalproperties/handler.go b/internal/additionalproperties/handler.go deleted file mode 100644 index 2b96cd0317..0000000000 --- a/internal/additionalproperties/handler.go +++ /dev/null @@ -1,114 +0,0 @@ -package additionalproperties - -import ( - "bufio" - "fmt" - "log/slog" - "net/http" - "os" - "path/filepath" - "strconv" - - "github.com/kyma-project/kyma-environment-broker/internal/httputil" -) - -const ( - ProvisioningRequestsFileName = "provisioning-requests.jsonl" - UpdateRequestsFileName = "update-requests.jsonl" -) - -type Handler struct { - logger *slog.Logger - additionalPropertiesPath string -} - -type Page struct { - Data []string `json:"data"` - Count int `json:"count"` - TotalCount int `json:"totalCount"` -} - -func NewHandler(logger *slog.Logger, additionalPropertiesPath string) *Handler { - return &Handler{ - logger: logger.With("service", "additional-properties-handler"), - additionalPropertiesPath: additionalPropertiesPath, - } -} - -func (h *Handler) AttachRoutes(router *httputil.Router) { - router.HandleFunc("/additional_properties", h.getAdditionalProperties) -} - -func (h *Handler) getAdditionalProperties(w http.ResponseWriter, req *http.Request) { - requestType := req.URL.Query().Get("requestType") - pageNumberStr := req.URL.Query().Get("pageNumber") - pageSizeStr := req.URL.Query().Get("pageSize") - - pageNumber := 0 - pageSize := 100 - - if pageNumberStr != "" { - if n, err := strconv.Atoi(pageNumberStr); err == nil && n >= 0 { - pageNumber = n - } - } - if pageSizeStr != "" { - if s, err := strconv.Atoi(pageSizeStr); err == nil && s > 0 { - pageSize = s - } - } - - var fileName string - switch requestType { - case "provisioning": - fileName = ProvisioningRequestsFileName - case "update": - fileName = UpdateRequestsFileName - case "": - info := map[string]string{ - "message": "Missing query parameter 'requestType'. Supported values are 'provisioning' and 'update'.", - } - httputil.WriteResponse(w, http.StatusBadRequest, info) - return - default: - info := map[string]string{ - "message": fmt.Sprintf("Unsupported requestType '%s'. Supported values are 'provisioning' and 'update'.", requestType), - } - httputil.WriteResponse(w, http.StatusBadRequest, info) - return - } - filePath := filepath.Join(h.additionalPropertiesPath, fileName) - - f, err := os.Open(filePath) - if err != nil { - h.logger.Error("Failed to open additional properties file", "error", err) - httputil.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Errorf("while opening additional properties file: %s", err.Error())) - return - } - defer f.Close() - - skip := pageNumber * pageSize - end := skip + pageSize - lineNumber := 0 - var pageData []string - scanner := bufio.NewScanner(f) - for scanner.Scan() { - if lineNumber >= skip && lineNumber < end { - pageData = append(pageData, string(scanner.Bytes())) - } - lineNumber++ - } - if err := scanner.Err(); err != nil { - h.logger.Error("Error reading additional properties file", "error", err) - httputil.WriteErrorResponse(w, http.StatusInternalServerError, fmt.Errorf("while reading additional properties file: %s", err.Error())) - return - } - - page := Page{ - Data: pageData, - Count: len(pageData), - TotalCount: lineNumber, - } - - httputil.WriteResponse(w, http.StatusOK, page) -} diff --git a/internal/additionalproperties/handler_test.go b/internal/additionalproperties/handler_test.go deleted file mode 100644 index b505529ba5..0000000000 --- a/internal/additionalproperties/handler_test.go +++ /dev/null @@ -1,212 +0,0 @@ -package additionalproperties - -import ( - "encoding/json" - "fmt" - "log/slog" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "testing" - - "github.com/kyma-project/kyma-environment-broker/internal/httputil" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestGetAdditionalProperties(t *testing.T) { - tempDir := t.TempDir() - - provisioningFile := filepath.Join(tempDir, ProvisioningRequestsFileName) - provisioningContent := `{"globalAccountID":"ga1","subAccountID":"sa1","instanceID":"id1","payload":{"key":"provisioning1"}}` - err := os.WriteFile(provisioningFile, []byte(provisioningContent), 0644) - require.NoError(t, err) - - updateFile := filepath.Join(tempDir, UpdateRequestsFileName) - updateContent := `{"globalAccountID":"ga2","subAccountID":"sa2","instanceID":"id2","payload":{"key":"update1"}}` - err = os.WriteFile(updateFile, []byte(updateContent), 0644) - require.NoError(t, err) - - log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelInfo})) - handler := NewHandler(log, tempDir) - - router := httputil.NewRouter() - handler.AttachRoutes(router) - - t.Run("returns provisioning requests", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties?requestType=provisioning", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - require.Equal(t, http.StatusOK, resp.Code) - - var page Page - err := json.Unmarshal(resp.Body.Bytes(), &page) - require.NoError(t, err) - require.Len(t, page.Data, 1) - assert.Equal(t, 1, page.Count) - assert.Equal(t, 1, page.TotalCount) - - var data map[string]interface{} - err = json.Unmarshal([]byte(page.Data[0]), &data) - require.NoError(t, err) - assert.Equal(t, "ga1", data["globalAccountID"]) - assert.Equal(t, "sa1", data["subAccountID"]) - assert.Equal(t, "id1", data["instanceID"]) - payload := data["payload"].(map[string]interface{}) - assert.Equal(t, "provisioning1", payload["key"]) - }) - - t.Run("returns update requests", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties?requestType=update", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - require.Equal(t, http.StatusOK, resp.Code) - - var page Page - err := json.Unmarshal(resp.Body.Bytes(), &page) - require.NoError(t, err) - require.Len(t, page.Data, 1) - assert.Equal(t, 1, page.Count) - assert.Equal(t, 1, page.TotalCount) - - var data map[string]interface{} - err = json.Unmarshal([]byte(page.Data[0]), &data) - require.NoError(t, err) - assert.Equal(t, "ga2", data["globalAccountID"]) - assert.Equal(t, "sa2", data["subAccountID"]) - assert.Equal(t, "id2", data["instanceID"]) - payload := data["payload"].(map[string]interface{}) - assert.Equal(t, "update1", payload["key"]) - }) - - t.Run("returns error for missing requestType", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - require.Equal(t, http.StatusBadRequest, resp.Code) - - var data map[string]string - err := json.Unmarshal(resp.Body.Bytes(), &data) - require.NoError(t, err) - assert.Contains(t, data["message"], "Missing query parameter") - }) - - t.Run("returns error for invalid requestType", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties?requestType=invalid", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - require.Equal(t, http.StatusBadRequest, resp.Code) - - var data map[string]string - err := json.Unmarshal(resp.Body.Bytes(), &data) - require.NoError(t, err) - assert.Contains(t, data["message"], "Unsupported requestType") - }) -} - -func TestGetAdditionalProperties_Paging(t *testing.T) { - tempDir := t.TempDir() - - provisioningFile := filepath.Join(tempDir, ProvisioningRequestsFileName) - var content string - for i := 1; i <= 5; i++ { - line := fmt.Sprintf(`{"globalAccountID":"ga%d","subAccountID":"sa%d","instanceID":"id%d","payload":{"key":"p%d"}}`, i, i, i, i) - content += line + "\n" - } - err := os.WriteFile(provisioningFile, []byte(content), 0644) - require.NoError(t, err) - - log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelInfo})) - handler := NewHandler(log, tempDir) - - router := httputil.NewRouter() - handler.AttachRoutes(router) - - t.Run("returns first page with pageSize=2", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties?requestType=provisioning&pageSize=2&pageNumber=0", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - require.Equal(t, http.StatusOK, resp.Code) - - var page Page - err := json.Unmarshal(resp.Body.Bytes(), &page) - require.NoError(t, err) - assert.Equal(t, 2, page.Count) - assert.Equal(t, 5, page.TotalCount) - - var data map[string]interface{} - err = json.Unmarshal([]byte(page.Data[0]), &data) - require.NoError(t, err) - assert.Equal(t, "ga1", data["globalAccountID"]) - - err = json.Unmarshal([]byte(page.Data[1]), &data) - require.NoError(t, err) - assert.Equal(t, "ga2", data["globalAccountID"]) - }) - - t.Run("returns second page with pageSize=2", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties?requestType=provisioning&pageSize=2&pageNumber=1", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - var page Page - err := json.Unmarshal(resp.Body.Bytes(), &page) - require.NoError(t, err) - assert.Equal(t, 2, page.Count) - assert.Equal(t, 5, page.TotalCount) - - var data map[string]interface{} - err = json.Unmarshal([]byte(page.Data[0]), &data) - require.NoError(t, err) - assert.Equal(t, "ga3", data["globalAccountID"]) - - err = json.Unmarshal([]byte(page.Data[1]), &data) - require.NoError(t, err) - assert.Equal(t, "ga4", data["globalAccountID"]) - }) - - t.Run("returns last page with 1 item", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties?requestType=provisioning&pageSize=2&pageNumber=2", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - var page Page - err := json.Unmarshal(resp.Body.Bytes(), &page) - require.NoError(t, err) - assert.Equal(t, 1, page.Count) - assert.Equal(t, 5, page.TotalCount) - - var data map[string]interface{} - err = json.Unmarshal([]byte(page.Data[0]), &data) - require.NoError(t, err) - assert.Equal(t, "ga5", data["globalAccountID"]) - }) - - t.Run("returns empty data for out-of-range page", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/additional_properties?requestType=provisioning&pageSize=2&pageNumber=3", nil) - resp := httptest.NewRecorder() - - router.ServeHTTP(resp, req) - - var page Page - err := json.Unmarshal(resp.Body.Bytes(), &page) - require.NoError(t, err) - assert.Equal(t, 0, page.Count) - assert.Equal(t, 5, page.TotalCount) - assert.Empty(t, page.Data) - }) -} diff --git a/internal/broker/broker.go b/internal/broker/broker.go index dc9a8aad36..df7c94ad9e 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -64,8 +64,6 @@ type Config struct { UseAdditionalOIDCSchema bool `envconfig:"default=false"` - MonitorAdditionalProperties bool `envconfig:"default=false"` - AdditionalPropertiesPath string `envconfig:"default=/additional-properties"` GardenerSeedsCacheConfigMapName string `envconfig:"default=gardener-seeds-cache"` } diff --git a/internal/broker/instance_create.go b/internal/broker/instance_create.go index eb6ab1b503..4307671d27 100644 --- a/internal/broker/instance_create.go +++ b/internal/broker/instance_create.go @@ -1,7 +1,6 @@ package broker import ( - "bytes" "context" "encoding/base64" "encoding/json" @@ -10,8 +9,6 @@ import ( "net" "net/http" "net/netip" - "os" - "path/filepath" "slices" "strings" @@ -20,7 +17,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/common/gardener" 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" "github.com/kyma-project/kyma-environment-broker/internal/euaccess" @@ -173,9 +169,6 @@ func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, de if err != nil { return domain.ProvisionedServiceSpec{}, apiresponses.NewFailureResponse(err, http.StatusBadRequest, "while extracting context") } - if b.config.MonitorAdditionalProperties { - b.monitorAdditionalProperties(instanceID, ersContext, details.RawParameters) - } if b.config.DisableSapConvergedCloud && details.PlanID == SapConvergedCloudPlanID { err := fmt.Errorf("%s", ConvergedCloudBlockedMsg) return domain.ProvisionedServiceSpec{}, apiresponses.NewFailureResponse(err, http.StatusBadRequest, ConvergedCloudBlockedMsg) @@ -770,18 +763,6 @@ func (b *ProvisionEndpoint) validateNetworking(parameters pkg.ProvisioningParame return err } -func (b *ProvisionEndpoint) monitorAdditionalProperties(instanceID string, ersContext internal.ERSContext, rawParameters json.RawMessage) { - var parameters pkg.ProvisioningParametersDTO - decoder := json.NewDecoder(bytes.NewReader(rawParameters)) - decoder.DisallowUnknownFields() - if err := decoder.Decode(¶meters); err == nil { - return - } - if err := insertRequest(instanceID, filepath.Join(b.config.AdditionalPropertiesPath, additionalproperties.ProvisioningRequestsFileName), ersContext, rawParameters); err != nil { - b.log.Error(fmt.Sprintf("failed to save provisioning request with additonal properties: %v", err)) - } -} - func (b *ProvisionEndpoint) validateSeedAndShootRegion(providerType, region string, supportedRegions []string, logger *slog.Logger) error { providerConfig := &internal.ProviderConfig{} if err := b.providerConfigProvider.Provide(providerType, providerConfig); err != nil { @@ -812,31 +793,6 @@ func (b *ProvisionEndpoint) filterOutUnsupportedSeedRegions(supportedRegions, se return supportedSeedRegions } -func insertRequest(instanceID, filePath string, ersContext internal.ERSContext, rawParameters json.RawMessage) error { - payload := map[string]interface{}{ - "globalAccountID": ersContext.GlobalAccountID, - "subAccountID": ersContext.SubAccountID, - "instanceID": instanceID, - "payload": rawParameters, - } - data, err := json.Marshal(payload) - if err != nil { - return fmt.Errorf("failed to marshal payload: %w", err) - } - - f, err := os.OpenFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, os.ModePerm) - if err != nil { - return fmt.Errorf("failed to open file: %w", err) - } - defer f.Close() - - if _, err := f.Write(append(data, '\n')); err != nil { - return fmt.Errorf("failed to write payload: %w", err) - } - - return nil -} - func validateOverlapping(n1 net.IPNet, n2 net.IPNet) error { if n1.Contains(n2.IP) || n2.Contains(n1.IP) { diff --git a/internal/broker/instance_create_test.go b/internal/broker/instance_create_test.go index a944b22255..8786dcdbd7 100644 --- a/internal/broker/instance_create_test.go +++ b/internal/broker/instance_create_test.go @@ -1,7 +1,6 @@ package broker_test import ( - "bytes" "context" "encoding/json" "errors" @@ -9,13 +8,11 @@ import ( "log/slog" "net/http" "os" - "path/filepath" "testing" "github.com/kyma-project/kyma-environment-broker/common/gardener" 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/broker" "github.com/kyma-project/kyma-environment-broker/internal/broker/automock" "github.com/kyma-project/kyma-environment-broker/internal/config" @@ -2660,223 +2657,6 @@ func TestAvailableZonesValidation(t *testing.T) { assert.EqualError(t, err, "In the us-east-1, the g6.xlarge machine type is not available in 3 zones. If you want to use this machine type, set HA to false.") } -func TestAdditionalProperties(t *testing.T) { - t.Run("file should contain request with additional properties", func(t *testing.T) { - // given - tempDir := t.TempDir() - expectedFile := filepath.Join(tempDir, additionalproperties.ProvisioningRequestsFileName) - - log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ - Level: slog.LevelInfo, - })) - - memoryStorage := storage.NewMemoryStorage() - - queue := &automock.Queue{} - queue.On("Add", mock.AnythingOfType("string")) - - factoryBuilder := &automock.PlanValidator{} - factoryBuilder.On("IsPlanSupport", broker.AWSPlanID).Return(true) - - kcBuilder := &kcMock.KcBuilder{} - kcBuilder.On("GetServerURL", "").Return("", fmt.Errorf("error")) - // #create provisioner endpoint - provisionEndpoint := broker.NewProvision( - broker.Config{ - EnablePlans: []string{"aws"}, - URL: brokerURL, - OnlySingleTrialPerGA: true, - MonitorAdditionalProperties: true, - AdditionalPropertiesPath: tempDir, - }, - gardener.Config{Project: "test", ShootDomain: "example.com", DNSProviders: fixDNSProviders()}, - imConfigFixture, - memoryStorage, - queue, - broker.PlansConfig{}, - log, - dashboardConfig, - kcBuilder, - whitelist.Set{}, - newSchemaService(t), - fixRegionsSupportingMachine(), - fixValueProvider(t), - false, - config.FakeProviderConfigProvider{}, - ) - - // when - _, err := provisionEndpoint.Provision(fixRequestContext(t, "cf-eu10"), instanceID, domain.ProvisionDetails{ - ServiceID: serviceID, - PlanID: broker.AWSPlanID, - RawParameters: json.RawMessage(fmt.Sprintf(`{"name": "%s", "region": "%s", "test": "test"}`, clusterName, "eu-central-1")), - RawContext: json.RawMessage(fmt.Sprintf(`{"globalaccount_id": "%s", "subaccount_id": "%s", "user_id": "%s"}`, "any-global-account-id", subAccountID, "Test@Test.pl")), - }, true) - t.Logf("%+v\n", *provisionEndpoint) - - // then - assert.NoError(t, err) - - contents, err := os.ReadFile(expectedFile) - assert.NoError(t, err) - - lines := bytes.Split(contents, []byte("\n")) - assert.Greater(t, len(lines), 0) - var entry map[string]interface{} - err = json.Unmarshal(lines[0], &entry) - assert.NoError(t, err) - - assert.Equal(t, "any-global-account-id", entry["globalAccountID"]) - assert.Equal(t, subAccountID, entry["subAccountID"]) - assert.Equal(t, instanceID, entry["instanceID"]) - }) - - t.Run("file should contain two requests with additional properties", func(t *testing.T) { - // given - tempDir := t.TempDir() - expectedFile := filepath.Join(tempDir, additionalproperties.ProvisioningRequestsFileName) - - log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ - Level: slog.LevelInfo, - })) - - memoryStorage := storage.NewMemoryStorage() - - queue := &automock.Queue{} - queue.On("Add", mock.AnythingOfType("string")) - - factoryBuilder := &automock.PlanValidator{} - factoryBuilder.On("IsPlanSupport", broker.AWSPlanID).Return(true) - - kcBuilder := &kcMock.KcBuilder{} - kcBuilder.On("GetServerURL", "").Return("", fmt.Errorf("error")) - // #create provisioner endpoint - provisionEndpoint := broker.NewProvision( - broker.Config{ - EnablePlans: []string{"aws"}, - URL: brokerURL, - OnlySingleTrialPerGA: true, - MonitorAdditionalProperties: true, - AdditionalPropertiesPath: tempDir, - }, - gardener.Config{Project: "test", ShootDomain: "example.com", DNSProviders: fixDNSProviders()}, - imConfigFixture, - memoryStorage, - queue, - broker.PlansConfig{}, - log, - dashboardConfig, - kcBuilder, - whitelist.Set{}, - newSchemaService(t), - fixRegionsSupportingMachine(), - fixValueProvider(t), - false, - config.FakeProviderConfigProvider{}, - ) - - // when - _, err := provisionEndpoint.Provision(fixRequestContext(t, "cf-eu10"), instanceID, domain.ProvisionDetails{ - ServiceID: serviceID, - PlanID: broker.AWSPlanID, - RawParameters: json.RawMessage(fmt.Sprintf(`{"name": "%s", "region": "%s", "test": "test"}`, clusterName, "eu-central-1")), - RawContext: json.RawMessage(fmt.Sprintf(`{"globalaccount_id": "%s", "subaccount_id": "%s", "user_id": "%s"}`, "any-global-account-id", subAccountID, "Test@Test.pl")), - }, true) - t.Logf("%+v\n", *provisionEndpoint) - assert.NoError(t, err) - - _, err = provisionEndpoint.Provision(fixRequestContext(t, "cf-eu10"), instanceID, domain.ProvisionDetails{ - ServiceID: serviceID, - PlanID: broker.AWSPlanID, - RawParameters: json.RawMessage(fmt.Sprintf(`{"name": "%s", "region": "%s", "test": "test"}`, clusterName, "eu-central-1")), - RawContext: json.RawMessage(fmt.Sprintf(`{"globalaccount_id": "%s", "subaccount_id": "%s", "user_id": "%s"}`, "any-global-account-id", subAccountID, "Test@Test.pl")), - }, true) - t.Logf("%+v\n", *provisionEndpoint) - assert.NoError(t, err) - - // then - contents, err := os.ReadFile(expectedFile) - assert.NoError(t, err) - - lines := bytes.Split(contents, []byte("\n")) - assert.Equal(t, len(lines), 3) - var entry map[string]interface{} - - err = json.Unmarshal(lines[0], &entry) - assert.NoError(t, err) - assert.Equal(t, "any-global-account-id", entry["globalAccountID"]) - assert.Equal(t, subAccountID, entry["subAccountID"]) - assert.Equal(t, instanceID, entry["instanceID"]) - - err = json.Unmarshal(lines[1], &entry) - assert.NoError(t, err) - assert.Equal(t, "any-global-account-id", entry["globalAccountID"]) - assert.Equal(t, subAccountID, entry["subAccountID"]) - assert.Equal(t, instanceID, entry["instanceID"]) - }) - - t.Run("file should not contain request without additional properties", func(t *testing.T) { - // given - tempDir := t.TempDir() - expectedFile := filepath.Join(tempDir, additionalproperties.ProvisioningRequestsFileName) - - log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ - Level: slog.LevelInfo, - })) - - memoryStorage := storage.NewMemoryStorage() - - queue := &automock.Queue{} - queue.On("Add", mock.AnythingOfType("string")) - - factoryBuilder := &automock.PlanValidator{} - factoryBuilder.On("IsPlanSupport", broker.AWSPlanID).Return(true) - - kcBuilder := &kcMock.KcBuilder{} - kcBuilder.On("GetServerURL", "").Return("", fmt.Errorf("error")) - // #create provisioner endpoint - provisionEndpoint := broker.NewProvision( - broker.Config{ - EnablePlans: []string{"aws"}, - URL: brokerURL, - OnlySingleTrialPerGA: true, - MonitorAdditionalProperties: true, - AdditionalPropertiesPath: tempDir, - }, - gardener.Config{Project: "test", ShootDomain: "example.com", DNSProviders: fixDNSProviders()}, - imConfigFixture, - memoryStorage, - queue, - broker.PlansConfig{}, - log, - dashboardConfig, - kcBuilder, - whitelist.Set{}, - newSchemaService(t), - fixRegionsSupportingMachine(), - fixValueProvider(t), - false, - config.FakeProviderConfigProvider{}, - ) - - // when - _, err := provisionEndpoint.Provision(fixRequestContext(t, "cf-eu10"), instanceID, domain.ProvisionDetails{ - ServiceID: serviceID, - PlanID: broker.AWSPlanID, - RawParameters: json.RawMessage(fmt.Sprintf(`{"name": "%s", "region": "%s"}`, clusterName, "eu-central-1")), - RawContext: json.RawMessage(fmt.Sprintf(`{"globalaccount_id": "%s", "subaccount_id": "%s", "user_id": "%s"}`, "any-global-account-id", subAccountID, "Test@Test.pl")), - }, true) - t.Logf("%+v\n", *provisionEndpoint) - - // then - assert.NoError(t, err) - - contents, err := os.ReadFile(expectedFile) - assert.Nil(t, contents) - assert.Error(t, err) - }) -} - func TestSameRegionForSeedAndShoot(t *testing.T) { log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ Level: slog.LevelInfo, diff --git a/internal/broker/instance_update.go b/internal/broker/instance_update.go index 42ee45c7a6..d60e05ae74 100644 --- a/internal/broker/instance_update.go +++ b/internal/broker/instance_update.go @@ -1,19 +1,16 @@ package broker import ( - "bytes" "context" "encoding/json" "fmt" "log/slog" "net/http" - "path/filepath" "strings" "time" 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/dashboard" "github.com/kyma-project/kyma-environment-broker/internal/kubeconfig" "github.com/kyma-project/kyma-environment-broker/internal/ptr" @@ -126,9 +123,6 @@ func (b *UpdateEndpoint) Update(_ context.Context, instanceID string, details do } logger.Info(fmt.Sprintf("Global account ID: %s active: %s", instance.GlobalAccountID, ptr.BoolAsString(ersContext.Active))) logger.Info(fmt.Sprintf("Received context: %s", marshallRawContext(hideSensitiveDataFromRawContext(details.RawContext)))) - if b.config.MonitorAdditionalProperties { - b.monitorAdditionalProperties(instanceID, ersContext, details.RawParameters) - } // validation of incoming input if err := b.validateWithJsonSchemaValidator(details, instance); err != nil { return domain.UpdateServiceSpec{}, apiresponses.NewFailureResponse(err, http.StatusBadRequest, "validation failed") @@ -473,15 +467,3 @@ func (b *UpdateEndpoint) getJsonSchemaValidator(provider pkg.CloudProvider, plan return validator.NewFromSchema(plan.Schemas.Instance.Update.Parameters) } - -func (b *UpdateEndpoint) monitorAdditionalProperties(instanceID string, ersContext internal.ERSContext, rawParameters json.RawMessage) { - var parameters internal.UpdatingParametersDTO - decoder := json.NewDecoder(bytes.NewReader(rawParameters)) - decoder.DisallowUnknownFields() - if err := decoder.Decode(¶meters); err == nil { - return - } - if err := insertRequest(instanceID, filepath.Join(b.config.AdditionalPropertiesPath, additionalproperties.UpdateRequestsFileName), ersContext, rawParameters); err != nil { - b.log.Error(fmt.Sprintf("failed to save update request with additonal properties: %v", err)) - } -} diff --git a/internal/broker/instance_update_test.go b/internal/broker/instance_update_test.go index f50503c0e0..b01954f2f4 100644 --- a/internal/broker/instance_update_test.go +++ b/internal/broker/instance_update_test.go @@ -1,20 +1,16 @@ package broker_test import ( - "bytes" "context" "encoding/json" "fmt" "net/http" - "os" - "path/filepath" "strings" "testing" "time" "github.com/kyma-project/kyma-environment-broker/internal/provider/configuration" - "github.com/kyma-project/kyma-environment-broker/internal/additionalproperties" "github.com/kyma-project/kyma-environment-broker/internal/broker" "github.com/kyma-project/kyma-environment-broker/internal/provider" "github.com/kyma-project/kyma-environment-broker/internal/regionssupportingmachine" @@ -1537,154 +1533,6 @@ func TestMachineTypeUpdateInAdditionalWorkerNodePools(t *testing.T) { assert.EqualError(t, err, "Machine type setting is permanent, and you cannot change it for the name-1 additional worker node pool") } -func TestUpdateAdditionalProperties(t *testing.T) { - t.Run("file should contain request with additional properties", func(t *testing.T) { - // given - tempDir := t.TempDir() - expectedFile := filepath.Join(tempDir, additionalproperties.UpdateRequestsFileName) - instance := fixture.FixInstance(instanceID) - st := storage.NewMemoryStorage() - err := st.Instances().Insert(instance) - require.NoError(t, err) - err = st.Operations().InsertProvisioningOperation(fixProvisioningOperation("provisioning01")) - require.NoError(t, err) - - handler := &handler{} - q := &automock.Queue{} - q.On("Add", mock.AnythingOfType("string")) - - kcBuilder := &kcMock.KcBuilder{} - kcBuilder.On("GetServerURL", mock.Anything).Return("https://kcp.example.dummy", nil) - svc := broker.NewUpdate(broker.Config{MonitorAdditionalProperties: true, AdditionalPropertiesPath: tempDir}, st, handler, true, true, false, q, broker.PlansConfig{}, - fixValueProvider(t), fixLogger(), dashboardConfig, kcBuilder, fakeKcpK8sClient, regionssupportingmachine.RegionsSupportingMachine{}, imConfigFixture, newSchemaService(t)) - - // when - _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ - ServiceID: "", - PlanID: broker.AzurePlanID, - RawParameters: json.RawMessage("{\"machineType\":\"Standard_D16s_v5\",\"test\":\"test\"}"), - PreviousValues: domain.PreviousValues{}, - RawContext: json.RawMessage("{\"globalaccount_id\":\"globalaccount_id_1\", \"subaccount_id\":\"subaccount_id_1\", \"active\":true}"), - MaintenanceInfo: nil, - }, true) - - // then - assert.NoError(t, err) - - contents, err := os.ReadFile(expectedFile) - assert.NoError(t, err) - - lines := bytes.Split(contents, []byte("\n")) - assert.Greater(t, len(lines), 0) - var entry map[string]interface{} - err = json.Unmarshal(lines[0], &entry) - assert.NoError(t, err) - - assert.Equal(t, "globalaccount_id_1", entry["globalAccountID"]) - assert.Equal(t, "subaccount_id_1", entry["subAccountID"]) - assert.Equal(t, instanceID, entry["instanceID"]) - }) - - t.Run("file should contain two requests with additional properties", func(t *testing.T) { - // given - tempDir := t.TempDir() - expectedFile := filepath.Join(tempDir, additionalproperties.UpdateRequestsFileName) - instance := fixture.FixInstance(instanceID) - st := storage.NewMemoryStorage() - err := st.Instances().Insert(instance) - require.NoError(t, err) - err = st.Operations().InsertProvisioningOperation(fixProvisioningOperation("provisioning01")) - require.NoError(t, err) - - handler := &handler{} - q := &automock.Queue{} - q.On("Add", mock.AnythingOfType("string")) - - kcBuilder := &kcMock.KcBuilder{} - kcBuilder.On("GetServerURL", mock.Anything).Return("https://kcp.example.dummy", nil) - svc := broker.NewUpdate(broker.Config{MonitorAdditionalProperties: true, AdditionalPropertiesPath: tempDir}, st, handler, true, true, false, q, broker.PlansConfig{}, - fixValueProvider(t), fixLogger(), dashboardConfig, kcBuilder, fakeKcpK8sClient, regionssupportingmachine.RegionsSupportingMachine{}, imConfigFixture, newSchemaService(t)) - - // when - _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ - ServiceID: "", - PlanID: broker.AzurePlanID, - RawParameters: json.RawMessage("{\"machineType\":\"Standard_D16s_v5\",\"test\":\"test\"}"), - PreviousValues: domain.PreviousValues{}, - RawContext: json.RawMessage("{\"globalaccount_id\":\"globalaccount_id_1\", \"subaccount_id\":\"subaccount_id_1\", \"active\":true}"), - MaintenanceInfo: nil, - }, true) - assert.NoError(t, err) - - _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ - ServiceID: "", - PlanID: broker.AzurePlanID, - RawParameters: json.RawMessage("{\"machineType\":\"Standard_D16s_v5\",\"test\":\"test\"}"), - PreviousValues: domain.PreviousValues{}, - RawContext: json.RawMessage("{\"globalaccount_id\":\"globalaccount_id_1\", \"subaccount_id\":\"subaccount_id_1\", \"active\":true}"), - MaintenanceInfo: nil, - }, true) - assert.NoError(t, err) - - // then - contents, err := os.ReadFile(expectedFile) - assert.NoError(t, err) - lines := bytes.Split(contents, []byte("\n")) - assert.Equal(t, len(lines), 3) - var entry map[string]interface{} - - err = json.Unmarshal(lines[0], &entry) - assert.NoError(t, err) - assert.Equal(t, "globalaccount_id_1", entry["globalAccountID"]) - assert.Equal(t, "subaccount_id_1", entry["subAccountID"]) - assert.Equal(t, instanceID, entry["instanceID"]) - - err = json.Unmarshal(lines[1], &entry) - assert.NoError(t, err) - assert.Equal(t, "globalaccount_id_1", entry["globalAccountID"]) - assert.Equal(t, "subaccount_id_1", entry["subAccountID"]) - assert.Equal(t, instanceID, entry["instanceID"]) - }) - - t.Run("file should not contain request without additional properties", func(t *testing.T) { - // given - tempDir := t.TempDir() - expectedFile := filepath.Join(tempDir, additionalproperties.UpdateRequestsFileName) - instance := fixture.FixInstance(instanceID) - st := storage.NewMemoryStorage() - err := st.Instances().Insert(instance) - require.NoError(t, err) - err = st.Operations().InsertProvisioningOperation(fixProvisioningOperation("provisioning01")) - require.NoError(t, err) - - handler := &handler{} - q := &automock.Queue{} - q.On("Add", mock.AnythingOfType("string")) - - kcBuilder := &kcMock.KcBuilder{} - kcBuilder.On("GetServerURL", mock.Anything).Return("https://kcp.example.dummy", nil) - svc := broker.NewUpdate(broker.Config{MonitorAdditionalProperties: true, AdditionalPropertiesPath: tempDir}, st, handler, true, true, false, q, broker.PlansConfig{}, - fixValueProvider(t), fixLogger(), dashboardConfig, kcBuilder, fakeKcpK8sClient, regionssupportingmachine.RegionsSupportingMachine{}, imConfigFixture, newSchemaService(t)) - - // when - _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ - ServiceID: "", - PlanID: broker.AzurePlanID, - RawParameters: json.RawMessage("{\"machineType\":\"Standard_D16s_v5\"}"), - PreviousValues: domain.PreviousValues{}, - RawContext: json.RawMessage("{\"globalaccount_id\":\"globalaccount_id_1\", \"subaccount_id\":\"subaccount_id_1\", \"active\":true}"), - MaintenanceInfo: nil, - }, true) - - // then - assert.NoError(t, err) - - contents, err := os.ReadFile(expectedFile) - assert.Nil(t, contents) - assert.Error(t, err) - }) -} - func fixValueProvider(t *testing.T) broker.ValuesProvider { planSpec, _ := configuration.NewPlanSpecifications(strings.NewReader("")) return provider.NewPlanSpecificValuesProvider( diff --git a/resources/keb/templates/additional-properties-pvc.yaml b/resources/keb/templates/additional-properties-pvc.yaml deleted file mode 100644 index d770c2213f..0000000000 --- a/resources/keb/templates/additional-properties-pvc.yaml +++ /dev/null @@ -1,15 +0,0 @@ -{{ if .Values.broker.monitorAdditionalProperties }} -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - name: {{ include "kyma-env-broker.fullname" . }}-additional-properties - labels: -{{ include "kyma-env-broker.labels" . | indent 4 }} -spec: - accessModes: - - ReadWriteOnce - resources: - requests: - storage: 5Gi - storageClassName: standard -{{ end }} diff --git a/resources/keb/templates/authorization-policy.yaml b/resources/keb/templates/authorization-policy.yaml index 0405855197..d6f0309765 100644 --- a/resources/keb/templates/authorization-policy.yaml +++ b/resources/keb/templates/authorization-policy.yaml @@ -290,20 +290,3 @@ spec: matchLabels: app.kubernetes.io/name: {{ include "kyma-env-broker.name" . }} app.kubernetes.io/instance: {{ .Values.namePrefix }} ---- -apiVersion: security.istio.io/v1beta1 -kind: AuthorizationPolicy -metadata: - name: istio-additional-properties - namespace: kcp-system -spec: - action: DENY - rules: - - to: - - operation: - paths: - - /additional_properties - selector: - matchLabels: - app.kubernetes.io/name: {{ include "kyma-env-broker.name" . }} - app.kubernetes.io/instance: {{ .Values.namePrefix }} diff --git a/resources/keb/templates/deployment.yaml b/resources/keb/templates/deployment.yaml index 8c335c0759..9ad5d1d03c 100644 --- a/resources/keb/templates/deployment.yaml +++ b/resources/keb/templates/deployment.yaml @@ -56,21 +56,6 @@ spec: mountPath: /tmp/profiler readOnly: false {{- end }} - {{- if .Values.broker.monitorAdditionalProperties }} - - name: additional-properties - command: - - sh - - -c - - "chmod 777 /additional-properties && sleep infinity" - securityContext: - runAsUser: 0 - image: europe-docker.pkg.dev/kyma-project/prod/alpine:v20250430-cb54844f - imagePullPolicy: Always - volumeMounts: - - name: keb-additional-properties - mountPath: /additional-properties - readOnly: false - {{- end }} - name: {{ include "kyma-env-broker.name" . }} image: "{{ .Values.global.images.container_registry.path }}/{{ .Values.global.images.kyma_environment_broker.dir }}kyma-environment-broker:{{ .Values.global.images.kyma_environment_broker.version }}" imagePullPolicy: {{ .Values.deployment.image.pullPolicy }} @@ -111,8 +96,6 @@ spec: value: "gardener-seeds-cache" - name: APP_BROKER_INCLUDE_ADDITIONAL_PARAMS_IN_SCHEMA value: "{{ .Values.broker.includeAdditionalParamsInSchema }}" - - name: APP_BROKER_MONITOR_ADDITIONAL_PROPERTIES - value: "{{ .Values.broker.monitorAdditionalProperties }}" - name: APP_BROKER_ONLY_ONE_FREE_PER_GA value: "{{ .Values.broker.onlyOneFreePerGA }}" - name: APP_BROKER_ONLY_SINGLE_TRIAL_PER_GA @@ -325,11 +308,6 @@ spec: mountPath: /tmp/profiler readOnly: false {{- end }} - {{- if .Values.broker.monitorAdditionalProperties }} - - name: keb-additional-properties - mountPath: /additional-properties - readOnly: false - {{- end }} {{- if and (eq .Values.global.database.embedded.enabled false) (eq .Values.global.database.cloudsqlproxy.enabled false)}} - name: cloudsql-sslrootcert mountPath: /secrets/cloudsql-sslrootcert @@ -388,8 +366,3 @@ spec: persistentVolumeClaim: claimName: {{ include "kyma-env-broker.fullname" . }}-profiler {{- end }} - {{- if .Values.broker.monitorAdditionalProperties }} - - name: keb-additional-properties - persistentVolumeClaim: - claimName: {{ include "kyma-env-broker.fullname" . }}-additional-properties - {{- end }} diff --git a/resources/keb/values.yaml b/resources/keb/values.yaml index e87f6fd229..ded37d0a89 100644 --- a/resources/keb/values.yaml +++ b/resources/keb/values.yaml @@ -154,8 +154,6 @@ broker: freeExpirationPeriod: 720h # If true, additional (advanced or less common) parameters are included in the provisioning schema for service plans includeAdditionalParamsInSchema: "false" - # If true, collects properties from the provisioning request that are not explicitly defined in the schema and stores them in persistent storage - monitorAdditionalProperties: false # If true, restricts each global account to only one free (freemium) Kyma runtime. # When enabled, provisioning another free environment for the same global account is blocked even if the previous one is deprovisioned onlyOneFreePerGA: "false"