From e1c00ec8ed034e96988a08c3cafada4fa3f7bbbc Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Fri, 7 Feb 2025 10:53:36 +0700 Subject: [PATCH 1/6] chore: remove endpoint which returns a decrypted instance --- pkg/instance/docs.go | 2 +- pkg/instance/handler.go | 50 ----------------------------------------- pkg/instance/router.go | 1 - swagger/swagger.yaml | 25 --------------------- 4 files changed, 1 insertion(+), 77 deletions(-) diff --git a/pkg/instance/docs.go b/pkg/instance/docs.go index 1faa8b7a..f27da641 100644 --- a/pkg/instance/docs.go +++ b/pkg/instance/docs.go @@ -30,7 +30,7 @@ type _ struct { Selector string `json:"selector"` } -// swagger:parameters deleteInstance findById findByIdDecrypted saveInstance pauseInstance resumeInstance resetInstance findDeploymentById deployDeployment deleteDeployment status instanceWithDetails instanceWithDecryptedDetails filestoreBackup +// swagger:parameters deleteInstance findById findByIdDecrypted saveInstance pauseInstance resumeInstance resetInstance findDeploymentById deployDeployment deleteDeployment status instanceWithDetails filestoreBackup type _ struct { // in: path // required: true diff --git a/pkg/instance/handler.go b/pkg/instance/handler.go index d4038666..09fde863 100644 --- a/pkg/instance/handler.go +++ b/pkg/instance/handler.go @@ -378,56 +378,6 @@ func (h Handler) InstanceWithDetails(c *gin.Context) { c.JSON(http.StatusOK, instance) } -// InstanceWithDecryptedDetails instance -func (h Handler) InstanceWithDecryptedDetails(c *gin.Context) { - // swagger:route PUT /instances/{id}/decrypted-details instanceWithDecryptedDetails - // - // Instance with decrypted details - // - // Returns the details of an instance including decrypted parameters - // - // Security: - // oauth2: - // - // responses: - // 202: - // 401: Error - // 403: Error - // 404: Error - // 415: Error - id, ok := handler.GetPathParameter(c, "id") - if !ok { - return - } - - ctx := c.Request.Context() - user, err := handler.GetUserFromContext(ctx) - if err != nil { - _ = c.Error(err) - return - } - - instance, err := h.instanceService.FindDecryptedDeploymentInstanceById(ctx, id) - if err != nil { - _ = c.Error(err) - return - } - - deployment, err := h.instanceService.FindDeploymentById(ctx, instance.DeploymentID) - if err != nil { - _ = c.Error(err) - return - } - - canWrite := handler.CanWriteDeployment(user, deployment) - if !canWrite { - _ = c.AbortWithError(http.StatusUnauthorized, fmt.Errorf("write access denied")) - return - } - - c.JSON(http.StatusOK, instance) -} - // Reset instance func (h Handler) Reset(c *gin.Context) { // swagger:route PUT /instances/{id}/reset resetInstance diff --git a/pkg/instance/router.go b/pkg/instance/router.go index 51d2a7b4..b3ddba26 100644 --- a/pkg/instance/router.go +++ b/pkg/instance/router.go @@ -17,7 +17,6 @@ func Routes(r *gin.Engine, authenticator gin.HandlerFunc, handler Handler) { tokenAuthenticationRouter.GET("/instances/:id/logs", handler.Logs) tokenAuthenticationRouter.GET("/instances/:id/status", handler.Status) tokenAuthenticationRouter.GET("/instances/:id/details", handler.InstanceWithDetails) - tokenAuthenticationRouter.GET("/instances/:id/decrypted-details", handler.InstanceWithDecryptedDetails) tokenAuthenticationRouter.POST("/deployments", handler.SaveDeployment) tokenAuthenticationRouter.GET("/deployments", handler.FindDeployments) diff --git a/swagger/swagger.yaml b/swagger/swagger.yaml index 852f3886..ee12d25c 100644 --- a/swagger/swagger.yaml +++ b/swagger/swagger.yaml @@ -1368,31 +1368,6 @@ paths: "200": $ref: '#/responses/Response' summary: Health status - /instances/{id}/decrypted-details: - put: - description: Returns the details of an instance including decrypted parameters - operationId: instanceWithDecryptedDetails - parameters: - - format: uint64 - in: path - name: id - required: true - type: integer - x-go-name: ID - responses: - "202": - description: "" - "401": - $ref: '#/responses/Error' - "403": - $ref: '#/responses/Error' - "404": - $ref: '#/responses/Error' - "415": - $ref: '#/responses/Error' - security: - - oauth2: [] - summary: Instance with decrypted details /instances/{id}/details: put: description: Returns the details of an instance including parameters From 79d797b3c87f8a1ad9ebb69136a16ffc02e64a05 Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Thu, 13 Feb 2025 12:10:56 +0700 Subject: [PATCH 2/6] chore: strip sensitive parameter values --- pkg/instance/handler.go | 79 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 7 deletions(-) diff --git a/pkg/instance/handler.go b/pkg/instance/handler.go index 09fde863..47cd3406 100644 --- a/pkg/instance/handler.go +++ b/pkg/instance/handler.go @@ -2,8 +2,9 @@ package instance import ( "bufio" - "context" "fmt" + "github.com/dhis2-sre/im-manager/pkg/group" + "github.com/dhis2-sre/im-manager/pkg/stack" "io" "net/http" @@ -14,8 +15,9 @@ import ( "github.com/gin-gonic/gin" ) -func NewHandler(groupService groupServiceHandler, instanceService *Service, defaultTTL uint) Handler { +func NewHandler(stackService stack.Service, groupService group.Service, instanceService *Service, defaultTTL uint) Handler { return Handler{ + stackService: stackService, groupService: groupService, instanceService: instanceService, defaultTTL: defaultTTL, @@ -23,15 +25,12 @@ func NewHandler(groupService groupServiceHandler, instanceService *Service, defa } type Handler struct { - groupService groupServiceHandler + stackService stack.Service + groupService group.Service instanceService *Service defaultTTL uint } -type groupServiceHandler interface { - Find(ctx context.Context, name string) (*model.Group, error) -} - func (h Handler) DeployDeployment(c *gin.Context) { // swagger:route POST /deployments/{id}/deploy deployDeployment // @@ -91,9 +90,40 @@ func (h Handler) DeployDeployment(c *gin.Context) { return } + err = h.stripDeploymentSensitiveParameterValues(deployment) + if err != nil { + _ = c.Error(err) + return + } + c.JSON(http.StatusOK, deployment) } +func (h Handler) stripDeploymentSensitiveParameterValues(deployment *model.Deployment) error { + for _, instance := range deployment.Instances { + err := h.stripInstanceSensitiveParameterValues(instance) + if err != nil { + return err + } + } + return nil +} + +func (h Handler) stripInstanceSensitiveParameterValues(instance *model.DeploymentInstance) error { + stack, err := h.stackService.Find(instance.StackName) + if err != nil { + return err + } + + for index, parameter := range instance.Parameters { + if stack.Parameters[parameter.ParameterName].Sensitive { + parameter.Value = "!!!redacted!!!" + instance.Parameters[index] = parameter + } + } + return nil +} + type SaveDeploymentRequest struct { Name string `json:"name" binding:"required,dns_rfc1035_label"` Description string `json:"description"` @@ -167,6 +197,12 @@ func (h Handler) SaveDeployment(c *gin.Context) { return } + err = h.stripDeploymentSensitiveParameterValues(deployment) + if err != nil { + _ = c.Error(err) + return + } + c.JSON(http.StatusCreated, deployment) } @@ -212,6 +248,12 @@ func (h Handler) FindDeploymentById(c *gin.Context) { return } + err = h.stripDeploymentSensitiveParameterValues(deployment) + if err != nil { + _ = c.Error(err) + return + } + c.JSON(http.StatusOK, deployment) } @@ -285,6 +327,12 @@ func (h Handler) SaveInstance(c *gin.Context) { return } + err = h.stripInstanceSensitiveParameterValues(instance) + if err != nil { + _ = c.Error(err) + return + } + c.JSON(http.StatusCreated, instance) } @@ -375,6 +423,12 @@ func (h Handler) InstanceWithDetails(c *gin.Context) { return } + err = h.stripInstanceSensitiveParameterValues(instance) + if err != nil { + _ = c.Error(err) + return + } + c.JSON(http.StatusOK, instance) } @@ -722,6 +776,17 @@ func (h Handler) FindDeployments(c *gin.Context) { return } + for i, group := range groupsWithDeployments { + for j, deployment := range group.Deployments { + err := h.stripDeploymentSensitiveParameterValues(deployment) + if err != nil { + _ = c.Error(err) + return + } + groupsWithDeployments[i].Deployments[j] = deployment + } + } + c.JSON(http.StatusOK, groupsWithDeployments) } From 708392f9cd8e62406685e0f1aea5b6dcdb02beb0 Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Thu, 13 Feb 2025 12:35:13 +0700 Subject: [PATCH 3/6] chore: imports --- pkg/instance/handler.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/instance/handler.go b/pkg/instance/handler.go index 47cd3406..208193df 100644 --- a/pkg/instance/handler.go +++ b/pkg/instance/handler.go @@ -3,11 +3,12 @@ package instance import ( "bufio" "fmt" - "github.com/dhis2-sre/im-manager/pkg/group" - "github.com/dhis2-sre/im-manager/pkg/stack" "io" "net/http" + "github.com/dhis2-sre/im-manager/pkg/group" + "github.com/dhis2-sre/im-manager/pkg/stack" + "github.com/dhis2-sre/im-manager/internal/errdef" "github.com/dhis2-sre/im-manager/internal/handler" From 6177eae898cb4be8be70e499f52d2c66b3c5f035 Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Thu, 13 Feb 2025 13:44:43 +0700 Subject: [PATCH 4/6] chore: fix test --- cmd/serve/main.go | 6 +++--- pkg/instance/handler.go | 10 +++++++--- pkg/instance/instance_integration_test.go | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/cmd/serve/main.go b/cmd/serve/main.go index 6f991a04..c3310e17 100644 --- a/cmd/serve/main.go +++ b/cmd/serve/main.go @@ -169,7 +169,7 @@ func run() (err error) { stackHandler := stack.NewHandler(stackService) - instanceHandler, err := newInstanceHandler(groupService, instanceService) + instanceHandler, err := newInstanceHandler(stackService, groupService, instanceService) if err != nil { return err } @@ -478,13 +478,13 @@ func newRabbitMQ() (rabbitMQConfig, error) { }, nil } -func newInstanceHandler(groupService *group.Service, instanceService *instance.Service) (instance.Handler, error) { +func newInstanceHandler(stackService stack.Service, groupService *group.Service, instanceService *instance.Service) (instance.Handler, error) { defaultTTL, err := requireEnvAsUint("DEFAULT_TTL") if err != nil { return instance.Handler{}, err } - return instance.NewHandler(groupService, instanceService, defaultTTL), nil + return instance.NewHandler(stackService, groupService, instanceService, defaultTTL), nil } func newDatabaseHandler(ctx context.Context, logger *slog.Logger, db *gorm.DB, groupService *group.Service, instanceService *instance.Service, stackService stack.Service) (database.Handler, error) { diff --git a/pkg/instance/handler.go b/pkg/instance/handler.go index 208193df..5f15ec21 100644 --- a/pkg/instance/handler.go +++ b/pkg/instance/handler.go @@ -2,11 +2,11 @@ package instance import ( "bufio" + "context" "fmt" "io" "net/http" - "github.com/dhis2-sre/im-manager/pkg/group" "github.com/dhis2-sre/im-manager/pkg/stack" "github.com/dhis2-sre/im-manager/internal/errdef" @@ -16,7 +16,7 @@ import ( "github.com/gin-gonic/gin" ) -func NewHandler(stackService stack.Service, groupService group.Service, instanceService *Service, defaultTTL uint) Handler { +func NewHandler(stackService stack.Service, groupService groupServiceHandler, instanceService *Service, defaultTTL uint) Handler { return Handler{ stackService: stackService, groupService: groupService, @@ -27,11 +27,15 @@ func NewHandler(stackService stack.Service, groupService group.Service, instance type Handler struct { stackService stack.Service - groupService group.Service + groupService groupServiceHandler instanceService *Service defaultTTL uint } +type groupServiceHandler interface { + Find(ctx context.Context, name string) (*model.Group, error) +} + func (h Handler) DeployDeployment(c *gin.Context) { // swagger:route POST /deployments/{id}/deploy deployDeployment // diff --git a/pkg/instance/instance_integration_test.go b/pkg/instance/instance_integration_test.go index d4a68f8e..5e98f2f1 100644 --- a/pkg/instance/instance_integration_test.go +++ b/pkg/instance/instance_integration_test.go @@ -92,7 +92,7 @@ func TestInstanceHandler(t *testing.T) { } client := inttest.SetupHTTPServer(t, func(engine *gin.Engine) { var twoDayTTL uint = 172800 - instanceHandler := instance.NewHandler(groupService, instanceService, twoDayTTL) + instanceHandler := instance.NewHandler(stackService, groupService, instanceService, twoDayTTL) instance.Routes(engine, authenticator, instanceHandler) databaseHandler := database.NewHandler(logger, databaseService, groupService, instanceService, stackService) From fd6eb58c83b3003b68414b81b7e289a327765c9f Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Thu, 13 Feb 2025 13:56:53 +0700 Subject: [PATCH 5/6] chore: remove test --- pkg/instance/instance_integration_test.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/pkg/instance/instance_integration_test.go b/pkg/instance/instance_integration_test.go index 5e98f2f1..4d8b42ab 100644 --- a/pkg/instance/instance_integration_test.go +++ b/pkg/instance/instance_integration_test.go @@ -217,23 +217,6 @@ func TestInstanceHandler(t *testing.T) { assert.NotEqual(t, parameters["REPLICA_COUNT"], "1") } - t.Log("Get deployment instance with decrypted details") - path = fmt.Sprintf("/instances/%d/decrypted-details", deploymentInstance.ID) - var decryptedInstance model.DeploymentInstance - client.GetJSON(t, path, &decryptedInstance, inttest.WithAuthToken("sometoken")) - assert.Equal(t, deploymentInstance.ID, decryptedInstance.ID) - assert.Equal(t, "group-name", decryptedInstance.GroupName) - assert.Equal(t, "whoami-go", decryptedInstance.StackName) - assert.Len(t, decryptedInstance.Parameters, 5) - expectedParameters := model.DeploymentInstanceParameters{ - "CHART_VERSION": {0, "", "", "0.9.0"}, - "IMAGE_PULL_POLICY": {0, "", "", "IfNotPresent"}, - "IMAGE_REPOSITORY": {0, "", "", "whoami-go"}, - "IMAGE_TAG": {0, "", "", "0.6.0"}, - "REPLICA_COUNT": {0, "", "", "1"}, - } - assert.EqualExportedValues(t, expectedParameters, decryptedInstance.Parameters) - t.Log("Deploy deployment") path = fmt.Sprintf("/deployments/%d/deploy", deployment.ID) client.Do(t, http.MethodPost, path, nil, http.StatusOK, inttest.WithAuthToken("sometoken")) From 93335a905a9c60b6157812a05cc2f0caec02acb4 Mon Sep 17 00:00:00 2001 From: Andreas Jensen Date: Thu, 13 Feb 2025 15:04:34 +0700 Subject: [PATCH 6/6] chore: use "***" to hide sensitive value --- pkg/instance/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instance/handler.go b/pkg/instance/handler.go index 5f15ec21..ff603705 100644 --- a/pkg/instance/handler.go +++ b/pkg/instance/handler.go @@ -122,7 +122,7 @@ func (h Handler) stripInstanceSensitiveParameterValues(instance *model.Deploymen for index, parameter := range instance.Parameters { if stack.Parameters[parameter.ParameterName].Sensitive { - parameter.Value = "!!!redacted!!!" + parameter.Value = "***" instance.Parameters[index] = parameter } }