From fe611482fe3fc638ebb7be3ee9141428e42d171b Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 1 Mar 2024 17:28:20 +0100 Subject: [PATCH 01/42] Add Skeleton transferCommitment endpoint --- internal/api/commitment.go | 2 ++ internal/api/core.go | 1 + 2 files changed, 3 insertions(+) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 33f45f48..a91b363d 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -434,3 +434,5 @@ func (p *v1Provider) DeleteProjectCommitment(w http.ResponseWriter, r *http.Requ }) w.WriteHeader(http.StatusNoContent) } + +func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) {} diff --git a/internal/api/core.go b/internal/api/core.go index 38cd9e36..9e9dea13 100644 --- a/internal/api/core.go +++ b/internal/api/core.go @@ -153,6 +153,7 @@ func (p *v1Provider) AddTo(r *mux.Router) { r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/new").HandlerFunc(p.CreateProjectCommitment) r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/can-confirm").HandlerFunc(p.CanConfirmNewProjectCommitment) r.Methods("DELETE").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{id}").HandlerFunc(p.DeleteProjectCommitment) + r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{id}/start-transfer").HandlerFunc(p.TransferCommitment) } // RequireJSON will parse the request body into the given data structure, or From fcaab341054fcbe880d5c917ac0d6de8bd54d9f5 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 1 Mar 2024 17:30:13 +0100 Subject: [PATCH 02/42] Add token check --- internal/api/commitment.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index a91b363d..e1b586a1 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -435,4 +435,10 @@ func (p *v1Provider) DeleteProjectCommitment(w http.ResponseWriter, r *http.Requ w.WriteHeader(http.StatusNoContent) } -func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) {} +func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) { + httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/commitments/:id/start-transfer") + token := p.CheckToken(r) + if !token.Require(w, "project:edit") { + return + } +} From e3c70cf09293761cd20fb3eceb55ca11bcd72f29 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 4 Mar 2024 16:38:04 +0100 Subject: [PATCH 03/42] Commitment: Implement commitment GET from DB. Test: Implement DB insert --- internal/api/commitment.go | 45 +++++++++++++++++++++++++++++++++ internal/api/commitment_test.go | 39 ++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index e1b586a1..ac62a617 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -90,6 +90,12 @@ var ( WHERE cs.type = $2 AND cr.name = $3 ) `) + updateCommitmentTransferState = sqlext.SimplifyWhitespace(` + UPDATE project_commitments SET = $1 WHERE capacitor_id = ( + SELECT capacitor_id FROM cluster_services cs JOIN cluster_resources cr ON cs.id = cr.service_id + WHERE cs.type = $2 AND cr.name = $3 + ) + `) ) // GetProjectCommitments handles GET /v1/domains/:domain_id/projects/:project_id/commitments. @@ -435,10 +441,49 @@ func (p *v1Provider) DeleteProjectCommitment(w http.ResponseWriter, r *http.Requ w.WriteHeader(http.StatusNoContent) } +// TransferCommitment handles POST /v1/domains/:id/projects/:id/commitments/:id/start-transfer func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) { httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/commitments/:id/start-transfer") token := p.CheckToken(r) if !token.Require(w, "project:edit") { return } + dbDomain := p.FindDomainFromRequest(w, r) + if dbDomain == nil { + return + } + dbProject := p.FindProjectFromRequest(w, r, dbDomain) + if dbProject == nil { + return + } + var parseTarget struct { + Request limesresources.Commitment `json:"commitment"` + } + if !RequireJSON(w, r, &parseTarget) { + return + } + //req := parseTarget.Request + + //load commitment + var dbCommitment db.ProjectCommitment + err := p.DB.SelectOne(&dbCommitment, findProjectCommitmentByIDQuery, mux.Vars(r)["id"], dbProject.ID) + if errors.Is(err, sql.ErrNoRows) { + http.Error(w, "no such commitment", http.StatusNotFound) + return + } else if respondwith.ErrorText(w, err) { + return + } + + var loc azResourceLocation + err = p.DB.QueryRow(findProjectAZResourceLocationByIDQuery, dbCommitment.AZResourceID). + Scan(&loc.ServiceType, &loc.ResourceName, &loc.AvailabilityZone) + if errors.Is(err, sql.ErrNoRows) { + //defense in depth: this should not happen because all the relevant tables are connected by FK constraints + http.Error(w, "no route to this commitment", http.StatusNotFound) + return + } else if respondwith.ErrorText(w, err) { + return + } + + respondwith.JSON(w, http.StatusAccepted, map[string]any{"success": true}) } diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index cd5e25c1..b8dc06d5 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -542,3 +542,42 @@ func TestDeleteCommitmentErrorCases(t *testing.T) { ExpectBody: assert.StringData("no such commitment\n"), }.Check(t, s.Handler) } + +func Test_TransferCommitment(t *testing.T) { + s := test.NewSetup(t, + test.WithDBFixtureFile("fixtures/start-data-commitments.sql"), + test.WithConfig(testCommitmentsYAML), + test.WithAPIHandler(NewV1API), + ) + + // create the transfer data + request := func(amount uint64, transferStatus string) assert.JSONObject { + return assert.JSONObject{ + "commitment": assert.JSONObject{ + "id": 1, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-one", + "amount": amount, + "duration": "1 hour", + "transfer_status": transferStatus, + "confirm_by": time.Now().Unix(), + }, + } + } + + assert.HTTPRequest{ + Method: http.MethodPost, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/new", + Body: request(200, ""), + ExpectStatus: http.StatusCreated, + }.Check(t, s.Handler) + + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", + ExpectStatus: 202, + ExpectBody: assert.JSONObject{"success": true}, + Body: request(300, "unlisted"), + }.Check(t, s.Handler) +} From 98aa4977a6821593107951a5df9afbe81ad2b538 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 4 Mar 2024 18:08:02 +0100 Subject: [PATCH 04/42] Test: define working request/response objects --- internal/api/commitment.go | 33 +++++++++++++++++------ internal/api/commitment_test.go | 47 ++++++++++++++++++++++----------- 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index ac62a617..b54a149c 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -19,6 +19,7 @@ package api import ( + "crypto/rand" "database/sql" "encoding/json" "errors" @@ -91,10 +92,7 @@ var ( ) `) updateCommitmentTransferState = sqlext.SimplifyWhitespace(` - UPDATE project_commitments SET = $1 WHERE capacitor_id = ( - SELECT capacitor_id FROM cluster_services cs JOIN cluster_resources cr ON cs.id = cr.service_id - WHERE cs.type = $2 AND cr.name = $3 - ) + UPDATE project_commitments SET transfer_status = $1, transfer_token = $2 WHERE id = $3 `) ) @@ -462,18 +460,26 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) if !RequireJSON(w, r, &parseTarget) { return } - //req := parseTarget.Request + req := parseTarget.Request + + // create a transfer token + // TODO: token generations has to be checked in test. + + // execute update + row, err := p.DB.Exec(updateCommitmentTransferState, req.TransferStatus, "hallo", req.ID) + if respondwith.ErrorText(w, err) { + return + } //load commitment var dbCommitment db.ProjectCommitment - err := p.DB.SelectOne(&dbCommitment, findProjectCommitmentByIDQuery, mux.Vars(r)["id"], dbProject.ID) + err = p.DB.SelectOne(&dbCommitment, findProjectCommitmentByIDQuery, mux.Vars(r)["id"], dbProject.ID) if errors.Is(err, sql.ErrNoRows) { http.Error(w, "no such commitment", http.StatusNotFound) return } else if respondwith.ErrorText(w, err) { return } - var loc azResourceLocation err = p.DB.QueryRow(findProjectAZResourceLocationByIDQuery, dbCommitment.AZResourceID). Scan(&loc.ServiceType, &loc.ResourceName, &loc.AvailabilityZone) @@ -485,5 +491,16 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) return } - respondwith.JSON(w, http.StatusAccepted, map[string]any{"success": true}) + c := p.convertCommitmentToDisplayForm(dbCommitment, loc) + fmt.Println(row) + respondwith.JSON(w, http.StatusAccepted, map[string]any{"commitment": c}) +} + +func (p *v1Provider) createToken() (string, error) { + tokenBytes := make([]byte, 12) + _, err := rand.Read(tokenBytes) + if err != nil { + return "", fmt.Errorf("could not generate token: %s", err.Error()) + } + return string(tokenBytes), nil } diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index b8dc06d5..af012aea 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -550,34 +550,51 @@ func Test_TransferCommitment(t *testing.T) { test.WithAPIHandler(NewV1API), ) + var confirmBy = time.Now().Unix() // create the transfer data - request := func(amount uint64, transferStatus string) assert.JSONObject { + req1 := func(transfer_status string) assert.JSONObject { return assert.JSONObject{ - "commitment": assert.JSONObject{ - "id": 1, - "service_type": "first", - "resource_name": "capacity", - "availability_zone": "az-one", - "amount": amount, - "duration": "1 hour", - "transfer_status": transferStatus, - "confirm_by": time.Now().Unix(), - }, + "id": 1, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-one", + "amount": 10, + "duration": "1 hour", + "transfer_status": transfer_status, + "transfer_token": "", + "confirm_by": confirmBy, } } + resp1 := assert.JSONObject{ + "id": 1, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-one", + "amount": 10, + "unit": "B", + "duration": "1 hour", + "created_at": s.Clock.Now().Unix(), + "creator_uuid": "uuid-for-alice", + "creator_name": "alice@Default", + "confirm_by": confirmBy, + "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + "transfer_status": "unlisted", + "transfer_token": "hallo", + } + assert.HTTPRequest{ Method: http.MethodPost, Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/new", - Body: request(200, ""), + Body: assert.JSONObject{"commitment": req1("")}, ExpectStatus: http.StatusCreated, }.Check(t, s.Handler) assert.HTTPRequest{ Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", - ExpectStatus: 202, - ExpectBody: assert.JSONObject{"success": true}, - Body: request(300, "unlisted"), + ExpectStatus: http.StatusAccepted, + ExpectBody: assert.JSONObject{"commitment": resp1}, + Body: assert.JSONObject{"commitment": req1("unlisted")}, }.Check(t, s.Handler) } From 51c0c8a60d5ac352fa4b1bddd735407e53fcaaf7 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 4 Mar 2024 18:23:42 +0100 Subject: [PATCH 05/42] Add event logging --- internal/api/commitment.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index b54a149c..fa17dc69 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -466,7 +466,7 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) // TODO: token generations has to be checked in test. // execute update - row, err := p.DB.Exec(updateCommitmentTransferState, req.TransferStatus, "hallo", req.ID) + _, err := p.DB.Exec(updateCommitmentTransferState, req.TransferStatus, "hallo", req.ID) if respondwith.ErrorText(w, err) { return } @@ -492,7 +492,13 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) } c := p.convertCommitmentToDisplayForm(dbCommitment, loc) - fmt.Println(row) + logAndPublishEvent(p.timeNow(), r, token, http.StatusAccepted, commitmentEventTarget{ + DomainID: dbDomain.UUID, + DomainName: dbDomain.Name, + ProjectID: dbProject.UUID, + ProjectName: dbProject.Name, + Commitment: c, + }) respondwith.JSON(w, http.StatusAccepted, map[string]any{"commitment": c}) } From dcea033ebe485663a8c0833d998a59594e541f8b Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 5 Mar 2024 11:34:12 +0100 Subject: [PATCH 06/42] Generate transfer token: Implement function to be used in unit test and production --- internal/api/commitment.go | 14 ++------------ internal/api/commitment_test.go | 4 +++- internal/api/core.go | 10 +++++++++- internal/api/utils.go | 11 +++++++++++ 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index fa17dc69..c149b2c6 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -19,7 +19,6 @@ package api import ( - "crypto/rand" "database/sql" "encoding/json" "errors" @@ -463,10 +462,10 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) req := parseTarget.Request // create a transfer token - // TODO: token generations has to be checked in test. + transferToken := p.generateTransferToken() // execute update - _, err := p.DB.Exec(updateCommitmentTransferState, req.TransferStatus, "hallo", req.ID) + _, err := p.DB.Exec(updateCommitmentTransferState, req.TransferStatus, transferToken, req.ID) if respondwith.ErrorText(w, err) { return } @@ -501,12 +500,3 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) }) respondwith.JSON(w, http.StatusAccepted, map[string]any{"commitment": c}) } - -func (p *v1Provider) createToken() (string, error) { - tokenBytes := make([]byte, 12) - _, err := rand.Read(tokenBytes) - if err != nil { - return "", fmt.Errorf("could not generate token: %s", err.Error()) - } - return string(tokenBytes), nil -} diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index af012aea..f0d174bf 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -550,6 +550,8 @@ func Test_TransferCommitment(t *testing.T) { test.WithAPIHandler(NewV1API), ) + var transferToken = test.GenerateDummyToken() + var confirmBy = time.Now().Unix() // create the transfer data req1 := func(transfer_status string) assert.JSONObject { @@ -580,7 +582,7 @@ func Test_TransferCommitment(t *testing.T) { "confirm_by": confirmBy, "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), "transfer_status": "unlisted", - "transfer_token": "hallo", + "transfer_token": transferToken, } assert.HTTPRequest{ diff --git a/internal/api/core.go b/internal/api/core.go index 9e9dea13..033643f5 100644 --- a/internal/api/core.go +++ b/internal/api/core.go @@ -70,12 +70,14 @@ type v1Provider struct { listProjectsMutex sync.Mutex //slots for test doubles timeNow func() time.Time + //identifies commitments that will be transferred to other projects. + generateTransferToken func() string } // NewV1API creates an httpapi.API that serves the Limes v1 API. // It also returns the VersionData for this API version which is needed for the // version advertisement on "GET /". -func NewV1API(cluster *core.Cluster, dbm *gorp.DbMap, tokenValidator gopherpolicy.Validator, timeNow func() time.Time) httpapi.API { +func NewV1API(cluster *core.Cluster, dbm *gorp.DbMap, tokenValidator gopherpolicy.Validator, timeNow func() time.Time, generateToken func() string) httpapi.API { p := &v1Provider{Cluster: cluster, DB: dbm, tokenValidator: tokenValidator, timeNow: timeNow} p.VersionData = VersionData{ Status: "CURRENT", @@ -92,6 +94,7 @@ func NewV1API(cluster *core.Cluster, dbm *gorp.DbMap, tokenValidator gopherpolic }, }, } + p.generateTransferToken = generateToken return p } @@ -110,6 +113,11 @@ func NewTokenValidator(provider *gophercloud.ProviderClient, eo gophercloud.Endp return &tv, err } +func (p *v1Provider) OverrideGenerateToken(generateTransferToken func() string) *v1Provider { + p.generateTransferToken = generateTransferToken + return p +} + // AddTo implements the httpapi.API interface. func (p *v1Provider) AddTo(r *mux.Router) { r.Methods("HEAD", "GET").Path("/").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/api/utils.go b/internal/api/utils.go index 6ecae47d..0060535d 100644 --- a/internal/api/utils.go +++ b/internal/api/utils.go @@ -19,6 +19,8 @@ package api import ( + "crypto/rand" + "encoding/hex" "time" "github.com/sapcc/go-api-declarations/limes" @@ -44,3 +46,12 @@ func unwrapOrDefault[T any](value *T, defaultValue T) T { } return *value } + +func GenerateToken() string { + tokenBytes := make([]byte, 12) + _, err := rand.Read(tokenBytes) + if err != nil { + panic(err.Error()) + } + return hex.EncodeToString(tokenBytes) +} From 6c43ca18fbd21346c9fdc478adbebef8c112238e Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 5 Mar 2024 11:40:39 +0100 Subject: [PATCH 07/42] Add missing changes from last commit --- internal/test/setup.go | 10 +++++++--- main.go | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/test/setup.go b/internal/test/setup.go index 1a2f3fd2..a17ed995 100644 --- a/internal/test/setup.go +++ b/internal/test/setup.go @@ -46,7 +46,7 @@ import ( type setupParams struct { DBFixtureFile string ConfigYAML string - APIBuilder func(*core.Cluster, *gorp.DbMap, gopherpolicy.Validator, func() time.Time) httpapi.API + APIBuilder func(*core.Cluster, *gorp.DbMap, gopherpolicy.Validator, func() time.Time, func() string) httpapi.API APIMiddlewares []httpapi.API } @@ -74,7 +74,7 @@ func WithConfig(yamlStr string) SetupOption { // Limes API. The `apiBuilder` function signature matches NewV1API(). We cannot // directly call this function because that would create an import cycle, so it // must be given by the caller here. -func WithAPIHandler(apiBuilder func(*core.Cluster, *gorp.DbMap, gopherpolicy.Validator, func() time.Time) httpapi.API, middlewares ...httpapi.API) SetupOption { +func WithAPIHandler(apiBuilder func(*core.Cluster, *gorp.DbMap, gopherpolicy.Validator, func() time.Time, func() string) httpapi.API, middlewares ...httpapi.API) SetupOption { return func(params *setupParams) { params.APIBuilder = apiBuilder params.APIMiddlewares = middlewares @@ -101,6 +101,10 @@ type Setup struct { Handler http.Handler } +func GenerateDummyToken() string { + return "dummyToken" +} + // NewSetup prepares most or all pieces of Keppel for a test. func NewSetup(t *testing.T, opts ...SetupOption) Setup { logg.ShowDebug = osext.GetenvBool("LIMES_DEBUG") @@ -144,7 +148,7 @@ func NewSetup(t *testing.T, opts ...SetupOption) Setup { if params.APIBuilder != nil { s.Handler = httpapi.Compose( append([]httpapi.API{ - params.APIBuilder(s.Cluster, s.DB, s.TokenValidator, s.Clock.Now), + params.APIBuilder(s.Cluster, s.DB, s.TokenValidator, s.Clock.Now, GenerateDummyToken), httpapi.WithoutLogging(), }, params.APIMiddlewares...)..., ) diff --git a/main.go b/main.go index b841f5d0..1d412d6d 100644 --- a/main.go +++ b/main.go @@ -214,7 +214,7 @@ func taskServe(cluster *core.Cluster, args []string, provider *gophercloud.Provi }) mux := http.NewServeMux() mux.Handle("/", httpapi.Compose( - api.NewV1API(cluster, dbm, tokenValidator, time.Now), + api.NewV1API(cluster, dbm, tokenValidator, time.Now, api.GenerateToken), pprofapi.API{IsAuthorized: pprofapi.IsRequestFromLocalhost}, httpapi.WithGlobalMiddleware(api.ForbidClusterIDHeader), httpapi.WithGlobalMiddleware(corsMiddleware.Handler), From 91bb53438c527a556b014ff6191097c6311162ac Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 5 Mar 2024 14:46:44 +0100 Subject: [PATCH 08/42] Finalize: start-transfer API --- internal/api/commitment.go | 72 ++++++++++++++++++++++++++++----- internal/api/commitment_test.go | 28 ++++++++++++- internal/api/core.go | 2 +- 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index c149b2c6..5a16fd16 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -93,6 +93,9 @@ var ( updateCommitmentTransferState = sqlext.SimplifyWhitespace(` UPDATE project_commitments SET transfer_status = $1, transfer_token = $2 WHERE id = $3 `) + setCommitmentSuperseded = sqlext.SimplifyWhitespace(` + UPDATE project_commitments SET superseded_at = $1 WHERE id = $2 + `) ) // GetProjectCommitments handles GET /v1/domains/:domain_id/projects/:project_id/commitments. @@ -439,7 +442,7 @@ func (p *v1Provider) DeleteProjectCommitment(w http.ResponseWriter, r *http.Requ } // TransferCommitment handles POST /v1/domains/:id/projects/:id/commitments/:id/start-transfer -func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) { +func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Request) { httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/commitments/:id/start-transfer") token := p.CheckToken(r) if !token.Require(w, "project:edit") { @@ -461,24 +464,57 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) } req := parseTarget.Request - // create a transfer token - transferToken := p.generateTransferToken() - - // execute update - _, err := p.DB.Exec(updateCommitmentTransferState, req.TransferStatus, transferToken, req.ID) - if respondwith.ErrorText(w, err) { - return - } + // TODO: Check if Transferstatus in enum of commitment struct //load commitment var dbCommitment db.ProjectCommitment - err = p.DB.SelectOne(&dbCommitment, findProjectCommitmentByIDQuery, mux.Vars(r)["id"], dbProject.ID) + err := p.DB.SelectOne(&dbCommitment, findProjectCommitmentByIDQuery, mux.Vars(r)["id"], dbProject.ID) if errors.Is(err, sql.ErrNoRows) { http.Error(w, "no such commitment", http.StatusNotFound) return } else if respondwith.ErrorText(w, err) { return } + + // Transfer whole commitment or a newly created, splitted one. + tx, err := p.DB.Begin() + if respondwith.ErrorText(w, err) { + return + } + defer sqlext.RollbackUnlessCommitted(tx) + if req.Amount >= dbCommitment.Amount { + transferToken := p.generateTransferToken() + _, err = tx.Exec(updateCommitmentTransferState, req.TransferStatus, transferToken, dbCommitment.ID) + if respondwith.ErrorText(w, err) { + return + } + dbCommitment.TransferStatus = req.TransferStatus + dbCommitment.TransferToken = transferToken + } else { + now := p.timeNow() + transferAmount := req.Amount + remainingAmount := dbCommitment.Amount - req.Amount + transferCommitment := p.splitCommitment(dbCommitment, transferAmount) + remainingCommitment := p.splitCommitment(dbCommitment, remainingAmount) + err = tx.Insert(&transferCommitment) + if respondwith.ErrorText(w, err) { + return + } + err = tx.Insert(&remainingCommitment) + if respondwith.ErrorText(w, err) { + return + } + _, err = tx.Exec(setCommitmentSuperseded, now, dbCommitment.ID) + if respondwith.ErrorText(w, err) { + return + } + dbCommitment = transferCommitment + } + err = tx.Commit() + if respondwith.ErrorText(w, err) { + return + } + var loc azResourceLocation err = p.DB.QueryRow(findProjectAZResourceLocationByIDQuery, dbCommitment.AZResourceID). Scan(&loc.ServiceType, &loc.ResourceName, &loc.AvailabilityZone) @@ -500,3 +536,19 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) }) respondwith.JSON(w, http.StatusAccepted, map[string]any{"commitment": c}) } + +func (p *v1Provider) splitCommitment(dbCommitment db.ProjectCommitment, amount uint64) db.ProjectCommitment { + now := p.timeNow() + return db.ProjectCommitment{ + AZResourceID: dbCommitment.AZResourceID, + Amount: amount, + Duration: dbCommitment.Duration, + CreatedAt: now, + CreatorUUID: dbCommitment.CreatorUUID, + CreatorName: dbCommitment.CreatorName, + ConfirmBy: dbCommitment.ConfirmBy, + ConfirmedAt: dbCommitment.ConfirmedAt, + ExpiresAt: dbCommitment.ExpiresAt, + PredecessorID: &dbCommitment.ID, + } +} diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index f0d174bf..6b56ec09 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -553,7 +553,7 @@ func Test_TransferCommitment(t *testing.T) { var transferToken = test.GenerateDummyToken() var confirmBy = time.Now().Unix() - // create the transfer data + // 1. TransferAmount >= CommitmentAmount req1 := func(transfer_status string) assert.JSONObject { return assert.JSONObject{ "id": 1, @@ -597,6 +597,30 @@ func Test_TransferCommitment(t *testing.T) { Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", ExpectStatus: http.StatusAccepted, ExpectBody: assert.JSONObject{"commitment": resp1}, - Body: assert.JSONObject{"commitment": req1("unlisted")}, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 10, "transfer_status": "unlisted"}}, + }.Check(t, s.Handler) + + // 2. TransferAmount < CommitmentAmount + resp2 := assert.JSONObject{ + "id": 2, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-one", + "amount": 9, + "unit": "B", + "duration": "1 hour", + "created_at": s.Clock.Now().Unix(), + "creator_uuid": "uuid-for-alice", + "creator_name": "alice@Default", + "confirm_by": confirmBy, + "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + } + + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", + ExpectStatus: http.StatusAccepted, + ExpectBody: assert.JSONObject{"commitment": resp2}, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 9, "transfer_status": "unlisted"}}, }.Check(t, s.Handler) } diff --git a/internal/api/core.go b/internal/api/core.go index 033643f5..9da379db 100644 --- a/internal/api/core.go +++ b/internal/api/core.go @@ -161,7 +161,7 @@ func (p *v1Provider) AddTo(r *mux.Router) { r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/new").HandlerFunc(p.CreateProjectCommitment) r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/can-confirm").HandlerFunc(p.CanConfirmNewProjectCommitment) r.Methods("DELETE").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{id}").HandlerFunc(p.DeleteProjectCommitment) - r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{id}/start-transfer").HandlerFunc(p.TransferCommitment) + r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{id}/start-transfer").HandlerFunc(p.StartCommitmentTransfer) } // RequireJSON will parse the request body into the given data structure, or From c8a94a8fac572c8955ed90241c8c123fec9398d0 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 5 Mar 2024 15:06:27 +0100 Subject: [PATCH 09/42] Add transferStatus and token to splitted commitment --- internal/api/commitment.go | 6 +++--- internal/api/commitment_test.go | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 5a16fd16..56ef7cd5 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -481,15 +481,13 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ if respondwith.ErrorText(w, err) { return } + transferToken := p.generateTransferToken() defer sqlext.RollbackUnlessCommitted(tx) if req.Amount >= dbCommitment.Amount { - transferToken := p.generateTransferToken() _, err = tx.Exec(updateCommitmentTransferState, req.TransferStatus, transferToken, dbCommitment.ID) if respondwith.ErrorText(w, err) { return } - dbCommitment.TransferStatus = req.TransferStatus - dbCommitment.TransferToken = transferToken } else { now := p.timeNow() transferAmount := req.Amount @@ -510,6 +508,8 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ } dbCommitment = transferCommitment } + dbCommitment.TransferStatus = req.TransferStatus + dbCommitment.TransferToken = transferToken err = tx.Commit() if respondwith.ErrorText(w, err) { return diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index 6b56ec09..8f10fe08 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -614,6 +614,8 @@ func Test_TransferCommitment(t *testing.T) { "creator_name": "alice@Default", "confirm_by": confirmBy, "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + "transfer_status": "unlisted", + "transfer_token": transferToken, } assert.HTTPRequest{ From 4cc8f6cd09eac0acf88a00886e25940042481d44 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 5 Mar 2024 16:47:36 +0100 Subject: [PATCH 10/42] Check: unlisted or public status and amount (greater zero) --- internal/api/commitment.go | 12 ++++++++++-- internal/api/commitment_test.go | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 56ef7cd5..e5a9ea70 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -464,7 +464,15 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ } req := parseTarget.Request - // TODO: Check if Transferstatus in enum of commitment struct + if req.TransferStatus != limesresources.CommitmentTransferStatusUnlisted && req.TransferStatus != limesresources.CommitmentTransferStatusPublic { + respondwith.JSON(w, http.StatusBadRequest, fmt.Sprintf("Invalid transfer_status code. Must be %s or %s.", limesresources.CommitmentTransferStatusUnlisted, limesresources.CommitmentTransferStatusPublic)) + return + } + + if req.Amount <= 0 { + respondwith.JSON(w, http.StatusBadRequest, "Delivered amount needs to be a positive value.") + return + } //load commitment var dbCommitment db.ProjectCommitment @@ -481,8 +489,8 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ if respondwith.ErrorText(w, err) { return } - transferToken := p.generateTransferToken() defer sqlext.RollbackUnlessCommitted(tx) + transferToken := p.generateTransferToken() if req.Amount >= dbCommitment.Amount { _, err = tx.Exec(updateCommitmentTransferState, req.TransferStatus, transferToken, dbCommitment.ID) if respondwith.ErrorText(w, err) { diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index 8f10fe08..a4ece608 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -614,7 +614,7 @@ func Test_TransferCommitment(t *testing.T) { "creator_name": "alice@Default", "confirm_by": confirmBy, "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), - "transfer_status": "unlisted", + "transfer_status": "public", "transfer_token": transferToken, } @@ -623,6 +623,14 @@ func Test_TransferCommitment(t *testing.T) { Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", ExpectStatus: http.StatusAccepted, ExpectBody: assert.JSONObject{"commitment": resp2}, - Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 9, "transfer_status": "unlisted"}}, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 9, "transfer_status": "public"}}, + }.Check(t, s.Handler) + + // 3. Negative Test + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", + ExpectStatus: http.StatusBadRequest, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 0, "transfer_status": "public"}}, }.Check(t, s.Handler) } From 811f866a8e2e7969797fbb9c1c6b8194adac5d29 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Wed, 6 Mar 2024 10:49:25 +0100 Subject: [PATCH 11/42] Add sekelton for transfer-commitment API --- internal/api/commitment.go | 17 ++++++++++++++++- internal/api/commitment_test.go | 24 ++++++++++++++++++++---- internal/api/core.go | 1 + 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index e5a9ea70..3890738c 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -441,7 +441,7 @@ func (p *v1Provider) DeleteProjectCommitment(w http.ResponseWriter, r *http.Requ w.WriteHeader(http.StatusNoContent) } -// TransferCommitment handles POST /v1/domains/:id/projects/:id/commitments/:id/start-transfer +// StartCommitmentTransfer handles POST /v1/domains/:id/projects/:id/commitments/:id/start-transfer func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Request) { httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/commitments/:id/start-transfer") token := p.CheckToken(r) @@ -560,3 +560,18 @@ func (p *v1Provider) splitCommitment(dbCommitment db.ProjectCommitment, amount u PredecessorID: &dbCommitment.ID, } } + +// TransferCommitment handles POST /v1/domains/{domain_id}/projects/{project_id}/transfer-commitment/{id}?token={token} +func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) { + httpapi.IdentifyEndpoint(r, "/v1/domains/{domain_id}/projects/{project_id}/transfer-commitment/{id}") + token := p.CheckToken(r) + if !token.Require(w, "project:edit") { + return + } + transferToken := r.URL.Query().Get("token") + if transferToken == "" { + respondwith.ErrorText(w, errors.New("No transfer token provided")) + } + + respondwith.JSON(w, http.StatusAccepted, "") +} diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index a4ece608..b9e5873d 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -543,7 +543,7 @@ func TestDeleteCommitmentErrorCases(t *testing.T) { }.Check(t, s.Handler) } -func Test_TransferCommitment(t *testing.T) { +func Test_StartCommitmentTransfer(t *testing.T) { s := test.NewSetup(t, test.WithDBFixtureFile("fixtures/start-data-commitments.sql"), test.WithConfig(testCommitmentsYAML), @@ -553,7 +553,7 @@ func Test_TransferCommitment(t *testing.T) { var transferToken = test.GenerateDummyToken() var confirmBy = time.Now().Unix() - // 1. TransferAmount >= CommitmentAmount + // TransferAmount >= CommitmentAmount req1 := func(transfer_status string) assert.JSONObject { return assert.JSONObject{ "id": 1, @@ -600,7 +600,7 @@ func Test_TransferCommitment(t *testing.T) { Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 10, "transfer_status": "unlisted"}}, }.Check(t, s.Handler) - // 2. TransferAmount < CommitmentAmount + // TransferAmount < CommitmentAmount resp2 := assert.JSONObject{ "id": 2, "service_type": "first", @@ -626,7 +626,7 @@ func Test_TransferCommitment(t *testing.T) { Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 9, "transfer_status": "public"}}, }.Check(t, s.Handler) - // 3. Negative Test + // Negative Test, Amount = 0. assert.HTTPRequest{ Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", @@ -634,3 +634,19 @@ func Test_TransferCommitment(t *testing.T) { Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 0, "transfer_status": "public"}}, }.Check(t, s.Handler) } + +func Test_TransferCommitment(t *testing.T) { + s := test.NewSetup(t, + test.WithDBFixtureFile("fixtures/start-data-commitments.sql"), + test.WithConfig(testCommitmentsYAML), + test.WithAPIHandler(NewV1API), + ) + + // No token provided + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/1", + ExpectStatus: http.StatusInternalServerError, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 0, "transfer_status": "public"}}, + }.Check(t, s.Handler) +} diff --git a/internal/api/core.go b/internal/api/core.go index 9da379db..50cdf7b2 100644 --- a/internal/api/core.go +++ b/internal/api/core.go @@ -162,6 +162,7 @@ func (p *v1Provider) AddTo(r *mux.Router) { r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/can-confirm").HandlerFunc(p.CanConfirmNewProjectCommitment) r.Methods("DELETE").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{id}").HandlerFunc(p.DeleteProjectCommitment) r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{id}/start-transfer").HandlerFunc(p.StartCommitmentTransfer) + r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/transfer-commitment/{id}").HandlerFunc(p.TransferCommitment) } // RequireJSON will parse the request body into the given data structure, or From 79a988fecac1ae9484150b2ffd2730f2591c5847 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Wed, 6 Mar 2024 14:48:38 +0100 Subject: [PATCH 12/42] Implement Transfer Commitment API logic --- internal/api/commitment.go | 100 ++++++++++++++++++++++++++++++-- internal/api/commitment_test.go | 70 +++++++++++++++++++++- 2 files changed, 163 insertions(+), 7 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 3890738c..c3e56003 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -83,6 +83,20 @@ var ( JOIN project_services ps ON pr.service_id = ps.id WHERE par.id = $1 `) + getCommitmentWithMatchingTransferToken = sqlext.SimplifyWhitespace(` + SELECT * FROM project_commitments WHERE transfer_token = $1 + `) + findTargetAZResourceIDBySourceID = sqlext.SimplifyWhitespace(` + SELECT par.id + FROM project_az_resources as par + JOIN project_resources pr ON par.resource_id = pr.id + JOIN project_services ps ON pr.service_id = ps.id + WHERE ps.project_id = $1 AND az = ( + SELECT az + FROM project_az_resources + WHERE id = $2 + ) + `) forceImmediateCapacityScrapeQuery = sqlext.SimplifyWhitespace(` UPDATE cluster_capacitors SET next_scrape_at = $1 WHERE capacitor_id = ( @@ -93,9 +107,12 @@ var ( updateCommitmentTransferState = sqlext.SimplifyWhitespace(` UPDATE project_commitments SET transfer_status = $1, transfer_token = $2 WHERE id = $3 `) - setCommitmentSuperseded = sqlext.SimplifyWhitespace(` + updateCommitmentSuperseded = sqlext.SimplifyWhitespace(` UPDATE project_commitments SET superseded_at = $1 WHERE id = $2 `) + updateCommitmentAZResourceID = sqlext.SimplifyWhitespace(` + UPDATE project_commitments SET az_resource_id = $1 WHERE id = $2 + `) ) // GetProjectCommitments handles GET /v1/domains/:domain_id/projects/:project_id/commitments. @@ -484,7 +501,7 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ return } - // Transfer whole commitment or a newly created, splitted one. + // Mark whole commitment or a newly created, splitted one as transferrable. tx, err := p.DB.Begin() if respondwith.ErrorText(w, err) { return @@ -510,7 +527,7 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ if respondwith.ErrorText(w, err) { return } - _, err = tx.Exec(setCommitmentSuperseded, now, dbCommitment.ID) + _, err = tx.Exec(updateCommitmentSuperseded, now, dbCommitment.ID) if respondwith.ErrorText(w, err) { return } @@ -563,15 +580,86 @@ func (p *v1Provider) splitCommitment(dbCommitment db.ProjectCommitment, amount u // TransferCommitment handles POST /v1/domains/{domain_id}/projects/{project_id}/transfer-commitment/{id}?token={token} func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) { - httpapi.IdentifyEndpoint(r, "/v1/domains/{domain_id}/projects/{project_id}/transfer-commitment/{id}") + httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/transfer-commitment/:id?token=:token") token := p.CheckToken(r) if !token.Require(w, "project:edit") { return } transferToken := r.URL.Query().Get("token") if transferToken == "" { - respondwith.ErrorText(w, errors.New("No transfer token provided")) + respondwith.ErrorText(w, errors.New("no transfer token provided")) + } + dbDomain := p.FindDomainFromRequest(w, r) + if dbDomain == nil { + return + } + targetProject := p.FindProjectFromRequest(w, r, dbDomain) + if targetProject == nil { + return + } + + // find commitment by transfer_token + var dbCommitment db.ProjectCommitment + err := p.DB.SelectOne(&dbCommitment, getCommitmentWithMatchingTransferToken, transferToken) + if errors.Is(err, sql.ErrNoRows) { + http.Error(w, "no matching commitment found", http.StatusNotFound) + return + } else if respondwith.ErrorText(w, err) { + return + } + + // get target AZ_RESOURCE_ID + var targetResourceID db.ProjectAZResourceID + err = p.DB.QueryRow(findTargetAZResourceIDBySourceID, targetProject.ID, dbCommitment.AZResourceID).Scan(&targetResourceID) + if respondwith.ErrorText(w, err) { + return + } + + tx, err := p.DB.Begin() + if respondwith.ErrorText(w, err) { + return + } + defer sqlext.RollbackUnlessCommitted(tx) + + // update AZ_RESOURCE_ID of commitment + _, err = tx.Exec(updateCommitmentAZResourceID, targetResourceID, dbCommitment.ID) + if respondwith.ErrorText(w, err) { + return } - respondwith.JSON(w, http.StatusAccepted, "") + // reset transfer_status and transfer_token + _, err = tx.Exec(updateCommitmentTransferState, "", "", dbCommitment.ID) + if respondwith.ErrorText(w, err) { + return + } + + err = tx.Commit() + if respondwith.ErrorText(w, err) { + return + } + + dbCommitment.TransferStatus = "" + dbCommitment.TransferToken = "" + + var loc azResourceLocation + err = p.DB.QueryRow(findProjectAZResourceLocationByIDQuery, dbCommitment.AZResourceID). + Scan(&loc.ServiceType, &loc.ResourceName, &loc.AvailabilityZone) + if errors.Is(err, sql.ErrNoRows) { + //defense in depth: this should not happen because all the relevant tables are connected by FK constraints + http.Error(w, "no route to this commitment", http.StatusNotFound) + return + } else if respondwith.ErrorText(w, err) { + return + } + + c := p.convertCommitmentToDisplayForm(dbCommitment, loc) + logAndPublishEvent(p.timeNow(), r, token, http.StatusAccepted, commitmentEventTarget{ + DomainID: dbDomain.UUID, + DomainName: dbDomain.Name, + ProjectID: targetProject.UUID, + ProjectName: targetProject.Name, + Commitment: c, + }) + + respondwith.JSON(w, http.StatusAccepted, map[string]any{"commitment": c}) } diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index b9e5873d..9cf98475 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -642,11 +642,79 @@ func Test_TransferCommitment(t *testing.T) { test.WithAPIHandler(NewV1API), ) + var confirmBy = time.Now().Unix() + var transferToken = test.GenerateDummyToken() + req1 := assert.JSONObject{ + "id": 1, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-one", + "amount": 10, + "duration": "1 hour", + "transfer_status": "", + "confirm_by": confirmBy, + } + + resp1 := assert.JSONObject{ + "id": 1, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-one", + "amount": 10, + "unit": "B", + "duration": "1 hour", + "created_at": s.Clock.Now().Unix(), + "creator_uuid": "uuid-for-alice", + "creator_name": "alice@Default", + "confirm_by": confirmBy, + "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + "transfer_status": "unlisted", + "transfer_token": transferToken, + } + + resp2 := assert.JSONObject{ + "id": 1, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-one", + "amount": 10, + "unit": "B", + "duration": "1 hour", + "created_at": s.Clock.Now().Unix(), + "creator_uuid": "uuid-for-alice", + "creator_name": "alice@Default", + "confirm_by": confirmBy, + "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + } + + // Transfer Commitment to target AZ_RESOURCE_ID (SOURCE_ID=3 TARGET_ID=17) + assert.HTTPRequest{ + Method: http.MethodPost, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/new", + Body: assert.JSONObject{"commitment": req1}, + ExpectStatus: http.StatusCreated, + }.Check(t, s.Handler) + + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", + ExpectStatus: http.StatusAccepted, + ExpectBody: assert.JSONObject{"commitment": resp1}, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 10, "transfer_status": "unlisted"}}, + }.Check(t, s.Handler) + + assert.HTTPRequest{ + Method: http.MethodPost, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1?token=dummyToken", + ExpectBody: assert.JSONObject{"commitment": resp2}, + ExpectStatus: http.StatusAccepted, + }.Check(t, s.Handler) + // No token provided assert.HTTPRequest{ Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/1", ExpectStatus: http.StatusInternalServerError, - Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 0, "transfer_status": "public"}}, }.Check(t, s.Handler) + } From b70b3de3371e4cc2155b148a72a04d9de3c9b0b4 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 11 Mar 2024 12:28:44 +0100 Subject: [PATCH 13/42] Only allow commitment transfer on confirmed commitments --- internal/api/commitment.go | 6 +++ internal/api/commitment_test.go | 94 +++++++++++++++++++++++++-------- 2 files changed, 78 insertions(+), 22 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index c3e56003..49334f2a 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -501,6 +501,12 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ return } + // reject commitments that are not confirmed yet. + if dbCommitment.ConfirmedAt == nil { + http.Error(w, "project needs to be confirmed in order to transfer it.", http.StatusUnprocessableEntity) + return + } + // Mark whole commitment or a newly created, splitted one as transferrable. tx, err := p.DB.Begin() if respondwith.ErrorText(w, err) { diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index 9cf98475..8273033d 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -51,6 +51,24 @@ const testCommitmentsYAML = ` - resource: first/capacity commitment_is_az_aware: true ` +const testCommitmentsYAMLWithoutMinConfirmDate = ` + availability_zones: [ az-one, az-two ] + discovery: + method: --test-static + services: + - service_type: first + type: --test-generic + - service_type: second + type: --test-generic + resource_behavior: + # the resources in "first" have commitments, the ones in "second" do not + - resource: first/.* + commitment_durations: ["1 hour", "2 hours"] + - resource: first/things + commitment_is_az_aware: false + - resource: first/capacity + commitment_is_az_aware: true +` func TestCommitmentLifecycleWithDelayedConfirmation(t *testing.T) { s := test.NewSetup(t, @@ -546,25 +564,24 @@ func TestDeleteCommitmentErrorCases(t *testing.T) { func Test_StartCommitmentTransfer(t *testing.T) { s := test.NewSetup(t, test.WithDBFixtureFile("fixtures/start-data-commitments.sql"), - test.WithConfig(testCommitmentsYAML), + test.WithConfig(testCommitmentsYAMLWithoutMinConfirmDate), test.WithAPIHandler(NewV1API), ) var transferToken = test.GenerateDummyToken() - var confirmBy = time.Now().Unix() + // Test on confirmed commitment should succeed. // TransferAmount >= CommitmentAmount req1 := func(transfer_status string) assert.JSONObject { return assert.JSONObject{ "id": 1, "service_type": "first", "resource_name": "capacity", - "availability_zone": "az-one", + "availability_zone": "az-two", "amount": 10, "duration": "1 hour", "transfer_status": transfer_status, "transfer_token": "", - "confirm_by": confirmBy, } } @@ -572,15 +589,15 @@ func Test_StartCommitmentTransfer(t *testing.T) { "id": 1, "service_type": "first", "resource_name": "capacity", - "availability_zone": "az-one", + "availability_zone": "az-two", "amount": 10, "unit": "B", "duration": "1 hour", "created_at": s.Clock.Now().Unix(), "creator_uuid": "uuid-for-alice", "creator_name": "alice@Default", - "confirm_by": confirmBy, - "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + "confirmed_at": 0, + "expires_at": 3600, "transfer_status": "unlisted", "transfer_token": transferToken, } @@ -605,15 +622,15 @@ func Test_StartCommitmentTransfer(t *testing.T) { "id": 2, "service_type": "first", "resource_name": "capacity", - "availability_zone": "az-one", + "availability_zone": "az-two", "amount": 9, "unit": "B", "duration": "1 hour", "created_at": s.Clock.Now().Unix(), "creator_uuid": "uuid-for-alice", "creator_name": "alice@Default", - "confirm_by": confirmBy, - "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + "confirmed_at": 0, + "expires_at": 3600, "transfer_status": "public", "transfer_token": transferToken, } @@ -626,6 +643,35 @@ func Test_StartCommitmentTransfer(t *testing.T) { Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 9, "transfer_status": "public"}}, }.Check(t, s.Handler) + // Test on unconfirmed commitment should fail. + // ID is 4, because 2 additional commitments were created previously. + var confirmBy = time.Now().Unix() + req2 := assert.JSONObject{ + "id": 4, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-two", + "amount": 10, + "duration": "1 hour", + "transfer_status": "", + "transfer_token": "", + "confirm_by": confirmBy, + } + + assert.HTTPRequest{ + Method: http.MethodPost, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/new", + Body: assert.JSONObject{"commitment": req2}, + ExpectStatus: http.StatusCreated, + }.Check(t, s.Handler) + + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/4/start-transfer", + ExpectStatus: http.StatusUnprocessableEntity, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 10, "transfer_status": "unlisted"}}, + }.Check(t, s.Handler) + // Negative Test, Amount = 0. assert.HTTPRequest{ Method: "POST", @@ -638,36 +684,34 @@ func Test_StartCommitmentTransfer(t *testing.T) { func Test_TransferCommitment(t *testing.T) { s := test.NewSetup(t, test.WithDBFixtureFile("fixtures/start-data-commitments.sql"), - test.WithConfig(testCommitmentsYAML), + test.WithConfig(testCommitmentsYAMLWithoutMinConfirmDate), test.WithAPIHandler(NewV1API), ) - var confirmBy = time.Now().Unix() var transferToken = test.GenerateDummyToken() req1 := assert.JSONObject{ "id": 1, "service_type": "first", "resource_name": "capacity", - "availability_zone": "az-one", + "availability_zone": "az-two", "amount": 10, "duration": "1 hour", "transfer_status": "", - "confirm_by": confirmBy, } resp1 := assert.JSONObject{ "id": 1, "service_type": "first", "resource_name": "capacity", - "availability_zone": "az-one", + "availability_zone": "az-two", "amount": 10, "unit": "B", "duration": "1 hour", "created_at": s.Clock.Now().Unix(), "creator_uuid": "uuid-for-alice", "creator_name": "alice@Default", - "confirm_by": confirmBy, - "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + "confirmed_at": 0, + "expires_at": 3600, "transfer_status": "unlisted", "transfer_token": transferToken, } @@ -676,15 +720,15 @@ func Test_TransferCommitment(t *testing.T) { "id": 1, "service_type": "first", "resource_name": "capacity", - "availability_zone": "az-one", + "availability_zone": "az-two", "amount": 10, "unit": "B", "duration": "1 hour", "created_at": s.Clock.Now().Unix(), "creator_uuid": "uuid-for-alice", "creator_name": "alice@Default", - "confirm_by": confirmBy, - "expires_at": s.Clock.Now().Add(time.Duration(confirmBy)*time.Second + 1*time.Hour).Unix(), + "confirmed_at": 0, + "expires_at": 3600, } // Transfer Commitment to target AZ_RESOURCE_ID (SOURCE_ID=3 TARGET_ID=17) @@ -705,16 +749,22 @@ func Test_TransferCommitment(t *testing.T) { assert.HTTPRequest{ Method: http.MethodPost, - Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1?token=dummyToken", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1?token=" + transferToken, ExpectBody: assert.JSONObject{"commitment": resp2}, ExpectStatus: http.StatusAccepted, }.Check(t, s.Handler) + // wrong token + assert.HTTPRequest{ + Method: http.MethodPost, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1?token=wrongToken", + ExpectStatus: http.StatusNotFound, + }.Check(t, s.Handler) + // No token provided assert.HTTPRequest{ Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/1", ExpectStatus: http.StatusInternalServerError, }.Check(t, s.Handler) - } From 30b6ad4cae5ec9e8793f0547cd423a7c475a228e Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 11 Mar 2024 13:40:34 +0100 Subject: [PATCH 14/42] Fix typo in message: project -> commitment --- internal/api/commitment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 49334f2a..ab009862 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -503,7 +503,7 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ // reject commitments that are not confirmed yet. if dbCommitment.ConfirmedAt == nil { - http.Error(w, "project needs to be confirmed in order to transfer it.", http.StatusUnprocessableEntity) + http.Error(w, "commitment needs to be confirmed in order to transfer it.", http.StatusUnprocessableEntity) return } From 1278203af433ed4f4cbf83c5df18e989a923ec25 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 11 Mar 2024 14:28:33 +0100 Subject: [PATCH 15/42] Update API documentation --- docs/users/api-spec-resources.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index 88bce661..1f49a05a 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -752,6 +752,28 @@ Returns 200 (OK) on success, and a JSON document like `{"result":true}` or `{"re The `result` field indicates whether this commitment can be created without a `confirm_by` attribute, that is, confirmed immediately upon creation. +### POST /v1/domains/:id/projects/:id/commitments/:id/start-transfer +Prepares a commitment to be transferred from a source project to a target project. Requires a project-admin token, and a request body that is a JSON document like: +```json +{ + "commitment": { + "amount": 100, + "transfer_status": "unlisted", + } +} +``` +If the amount to transfer is equal to the commitment, the whole commitment will be marked as transferrable. If the amount is less than the commitment, the commitment will be split in two and the requested amount will be marked as transferrable.The transfer status indicates if the commitment stays `unlisted` (private) or `public`. The response is a JSON of the commitment including the following fields that identify a commitment in it's transferrable state: +```json +{ + "commitment": { + "transfer_token": "token", + "transfer_status": "unlisted", + } +} +``` +### POST /v1/domains/:id/projects/:id/transfer-commitment/:id?token=:token +Transfers the commitment from a source project to a target project. This endpoint receives the target project ID, but the commitment ID from the source project. Requires a generated token from the API: `/v1/domains/:id/projects/:id/commitments/:id/start-transfer`. On success the API clears the `transfer_token` and `transfer_status` from the commitment. After that, it returns the commitment as a JSON document. + ### DELETE /v1/domains/:domain\_id/projects/:project\_id/commitments/:id Deletes a commitment within the given project. Requires a cloud-admin token. On success, returns 204 (No Content). From 94c4816fa5147ce731db1921f6096ce724955123 Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Mon, 11 Mar 2024 15:27:22 +0100 Subject: [PATCH 16/42] Update docs/users/api-spec-resources.md Co-authored-by: Sandro --- docs/users/api-spec-resources.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index 1f49a05a..1357b168 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -758,7 +758,7 @@ Prepares a commitment to be transferred from a source project to a target projec { "commitment": { "amount": 100, - "transfer_status": "unlisted", + "transfer_status": "unlisted" } } ``` From f4e542343016286294501d2d77430f93a75505ec Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Mon, 11 Mar 2024 15:27:38 +0100 Subject: [PATCH 17/42] Update docs/users/api-spec-resources.md Co-authored-by: Sandro --- docs/users/api-spec-resources.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index 1357b168..08f96e5a 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -762,7 +762,9 @@ Prepares a commitment to be transferred from a source project to a target projec } } ``` -If the amount to transfer is equal to the commitment, the whole commitment will be marked as transferrable. If the amount is less than the commitment, the commitment will be split in two and the requested amount will be marked as transferrable.The transfer status indicates if the commitment stays `unlisted` (private) or `public`. The response is a JSON of the commitment including the following fields that identify a commitment in it's transferrable state: +If the amount to transfer is equal to the commitment, the whole commitment will be marked as transferrable. If the amount is less than the commitment, the commitment will be split in two and the requested amount will be marked as transferrable. +The transfer status indicates if the commitment stays `unlisted` (private) or `public`. +The response is a JSON of the commitment including the following fields that identify a commitment in it's transferrable state: ```json { "commitment": { From 810c25a01652cdaa2209799f908bf5b6228f191b Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Mon, 11 Mar 2024 15:27:45 +0100 Subject: [PATCH 18/42] Update docs/users/api-spec-resources.md Co-authored-by: Sandro --- docs/users/api-spec-resources.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index 08f96e5a..8e0245d9 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -769,7 +769,7 @@ The response is a JSON of the commitment including the following fields that ide { "commitment": { "transfer_token": "token", - "transfer_status": "unlisted", + "transfer_status": "unlisted" } } ``` From fe53bb58c4c256b52f7e108c4f167cb94ec98131 Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Mon, 11 Mar 2024 15:27:57 +0100 Subject: [PATCH 19/42] Update docs/users/api-spec-resources.md Co-authored-by: Sandro --- docs/users/api-spec-resources.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index 8e0245d9..0960e31d 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -774,7 +774,11 @@ The response is a JSON of the commitment including the following fields that ide } ``` ### POST /v1/domains/:id/projects/:id/transfer-commitment/:id?token=:token -Transfers the commitment from a source project to a target project. This endpoint receives the target project ID, but the commitment ID from the source project. Requires a generated token from the API: `/v1/domains/:id/projects/:id/commitments/:id/start-transfer`. On success the API clears the `transfer_token` and `transfer_status` from the commitment. After that, it returns the commitment as a JSON document. +Transfers the commitment from a source project to a target project. +This endpoint receives the target project ID, but the commitment ID from the source project. +Requires a generated token from the API: `/v1/domains/:id/projects/:id/commitments/:id/start-transfer`. +On success the API clears the `transfer_token` and `transfer_status` from the commitment. +After that, it returns the commitment as a JSON document. ### DELETE /v1/domains/:domain\_id/projects/:project\_id/commitments/:id From 42b6992cbb0024d6d45f063872f974040b839c68 Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Mon, 11 Mar 2024 15:28:18 +0100 Subject: [PATCH 20/42] Update internal/api/utils.go Co-authored-by: Sandro --- internal/api/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/utils.go b/internal/api/utils.go index 0060535d..a1b3cb99 100644 --- a/internal/api/utils.go +++ b/internal/api/utils.go @@ -48,7 +48,7 @@ func unwrapOrDefault[T any](value *T, defaultValue T) T { } func GenerateToken() string { - tokenBytes := make([]byte, 12) + tokenBytes := make([]byte, 24) _, err := rand.Read(tokenBytes) if err != nil { panic(err.Error()) From 94703ba1c01d102e775ebf744bee49c25e67b9fa Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 11 Mar 2024 15:33:04 +0100 Subject: [PATCH 21/42] Return error if project not found --- internal/api/commitment.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index ab009862..1a2e2e55 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -471,6 +471,7 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ } dbProject := p.FindProjectFromRequest(w, r, dbDomain) if dbProject == nil { + http.Error(w, "project not found.", http.StatusNotFound) return } var parseTarget struct { From d4c36353ff19ee00bdb7398a80a0f8214a5e528c Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 11 Mar 2024 15:40:59 +0100 Subject: [PATCH 22/42] Tests: Change time usage to clock usage --- internal/api/commitment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index 8273033d..eaec7db5 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -645,7 +645,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { // Test on unconfirmed commitment should fail. // ID is 4, because 2 additional commitments were created previously. - var confirmBy = time.Now().Unix() + var confirmBy = s.Clock.Now().Unix() req2 := assert.JSONObject{ "id": 4, "service_type": "first", From ddc1bcaaf58644b15d3fdb2e43a6d9cd59a28602 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Mon, 11 Mar 2024 15:57:00 +0100 Subject: [PATCH 23/42] Add missing error handlings --- docs/users/api-spec-resources.md | 1 + internal/api/commitment.go | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index 0960e31d..a25a8831 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -775,6 +775,7 @@ The response is a JSON of the commitment including the following fields that ide ``` ### POST /v1/domains/:id/projects/:id/transfer-commitment/:id?token=:token Transfers the commitment from a source project to a target project. +Requires a project-admin token. This endpoint receives the target project ID, but the commitment ID from the source project. Requires a generated token from the API: `/v1/domains/:id/projects/:id/commitments/:id/start-transfer`. On success the API clears the `transfer_token` and `transfer_status` from the commitment. diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 1a2e2e55..b7afcfbd 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -463,10 +463,12 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/commitments/:id/start-transfer") token := p.CheckToken(r) if !token.Require(w, "project:edit") { + http.Error(w, "insufficient access rights.", http.StatusForbidden) return } dbDomain := p.FindDomainFromRequest(w, r) if dbDomain == nil { + http.Error(w, "domain not found.", http.StatusNotFound) return } dbProject := p.FindProjectFromRequest(w, r, dbDomain) @@ -478,6 +480,7 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ Request limesresources.Commitment `json:"commitment"` } if !RequireJSON(w, r, &parseTarget) { + http.Error(w, "json not parsable.", http.StatusBadRequest) return } req := parseTarget.Request @@ -590,6 +593,7 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/transfer-commitment/:id?token=:token") token := p.CheckToken(r) if !token.Require(w, "project:edit") { + http.Error(w, "insufficient access rights.", http.StatusForbidden) return } transferToken := r.URL.Query().Get("token") @@ -598,10 +602,12 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) } dbDomain := p.FindDomainFromRequest(w, r) if dbDomain == nil { + http.Error(w, "domain not found.", http.StatusNotFound) return } targetProject := p.FindProjectFromRequest(w, r, dbDomain) if targetProject == nil { + http.Error(w, "project not found.", http.StatusNotFound) return } From 2f94e4753f5433cf149a8234c774ec20edf7343e Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:26:30 +0100 Subject: [PATCH 24/42] Update docs/users/api-spec-resources.md Documentation: Fix grammatical indistinctness. Co-authored-by: Stefan Majewsky --- docs/users/api-spec-resources.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index a25a8831..878ffaa9 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -764,7 +764,7 @@ Prepares a commitment to be transferred from a source project to a target projec ``` If the amount to transfer is equal to the commitment, the whole commitment will be marked as transferrable. If the amount is less than the commitment, the commitment will be split in two and the requested amount will be marked as transferrable. The transfer status indicates if the commitment stays `unlisted` (private) or `public`. -The response is a JSON of the commitment including the following fields that identify a commitment in it's transferrable state: +The response is a JSON of the commitment including the following fields that identify a commitment in its transferrable state: ```json { "commitment": { From 6c3fbe83f7494bab2f43fbb3d6c44946d6d482f7 Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:29:36 +0100 Subject: [PATCH 25/42] Update internal/api/commitment.go Nounify variable names of SQL queries. Co-authored-by: Stefan Majewsky --- internal/api/commitment.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index b7afcfbd..c4b129d8 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -83,10 +83,10 @@ var ( JOIN project_services ps ON pr.service_id = ps.id WHERE par.id = $1 `) - getCommitmentWithMatchingTransferToken = sqlext.SimplifyWhitespace(` + getCommitmentWithMatchingTransferTokenQuery = sqlext.SimplifyWhitespace(` SELECT * FROM project_commitments WHERE transfer_token = $1 `) - findTargetAZResourceIDBySourceID = sqlext.SimplifyWhitespace(` + findTargetAZResourceIDBySourceIDQuery = sqlext.SimplifyWhitespace(` SELECT par.id FROM project_az_resources as par JOIN project_resources pr ON par.resource_id = pr.id From 66b7e54732d3f6782acdcc528d19b114e406c379 Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:30:20 +0100 Subject: [PATCH 26/42] Update internal/api/commitment.go Token check: Remove error return message. Already handled by token.Require function. Co-authored-by: Stefan Majewsky --- internal/api/commitment.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index c4b129d8..4275b3ac 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -463,7 +463,6 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/commitments/:id/start-transfer") token := p.CheckToken(r) if !token.Require(w, "project:edit") { - http.Error(w, "insufficient access rights.", http.StatusForbidden) return } dbDomain := p.FindDomainFromRequest(w, r) From ccc016da63885e2c9b6d14f20516cf25013151de Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:34:23 +0100 Subject: [PATCH 27/42] Update internal/api/commitment.go Rename: splitCommitment function to buildSplitCommitment. Co-authored-by: Stefan Majewsky --- internal/api/commitment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 4275b3ac..a7e6d273 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -571,7 +571,7 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ respondwith.JSON(w, http.StatusAccepted, map[string]any{"commitment": c}) } -func (p *v1Provider) splitCommitment(dbCommitment db.ProjectCommitment, amount uint64) db.ProjectCommitment { +func (p *v1Provider) buildSplitCommitment(dbCommitment db.ProjectCommitment, amount uint64) db.ProjectCommitment { now := p.timeNow() return db.ProjectCommitment{ AZResourceID: dbCommitment.AZResourceID, From 726f2481925babe654e3b49d9d69f99ba31d3d92 Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:35:34 +0100 Subject: [PATCH 28/42] Update internal/api/commitment_test.go Commitment test: Update parameter from snake case to camel case. Co-authored-by: Stefan Majewsky --- internal/api/commitment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index eaec7db5..6e2bc39a 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -572,7 +572,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { // Test on confirmed commitment should succeed. // TransferAmount >= CommitmentAmount - req1 := func(transfer_status string) assert.JSONObject { + req1 := func(transferStatus string) assert.JSONObject { return assert.JSONObject{ "id": 1, "service_type": "first", From 00d9ddb2a22f6b5301f79874aa1f316349933e34 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 13:37:56 +0100 Subject: [PATCH 29/42] Fix: Remaining Delta from the PR code suggestions --- internal/api/commitment.go | 8 ++++---- internal/api/commitment_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index a7e6d273..87a9addc 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -526,8 +526,8 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ now := p.timeNow() transferAmount := req.Amount remainingAmount := dbCommitment.Amount - req.Amount - transferCommitment := p.splitCommitment(dbCommitment, transferAmount) - remainingCommitment := p.splitCommitment(dbCommitment, remainingAmount) + transferCommitment := p.buildSplitCommitment(dbCommitment, transferAmount) + remainingCommitment := p.buildSplitCommitment(dbCommitment, remainingAmount) err = tx.Insert(&transferCommitment) if respondwith.ErrorText(w, err) { return @@ -612,7 +612,7 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) // find commitment by transfer_token var dbCommitment db.ProjectCommitment - err := p.DB.SelectOne(&dbCommitment, getCommitmentWithMatchingTransferToken, transferToken) + err := p.DB.SelectOne(&dbCommitment, getCommitmentWithMatchingTransferTokenQuery, transferToken) if errors.Is(err, sql.ErrNoRows) { http.Error(w, "no matching commitment found", http.StatusNotFound) return @@ -622,7 +622,7 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) // get target AZ_RESOURCE_ID var targetResourceID db.ProjectAZResourceID - err = p.DB.QueryRow(findTargetAZResourceIDBySourceID, targetProject.ID, dbCommitment.AZResourceID).Scan(&targetResourceID) + err = p.DB.QueryRow(findTargetAZResourceIDBySourceIDQuery, targetProject.ID, dbCommitment.AZResourceID).Scan(&targetResourceID) if respondwith.ErrorText(w, err) { return } diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index 6e2bc39a..fabe8756 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -580,7 +580,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { "availability_zone": "az-two", "amount": 10, "duration": "1 hour", - "transfer_status": transfer_status, + "transfer_status": transferStatus, "transfer_token": "", } } From 565233716d25713b036c8cb5d3bae5be70e7daec Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 14:17:30 +0100 Subject: [PATCH 30/42] Change naming: GenerateToken to GenerateTransferToken --- internal/api/core.go | 6 +++--- internal/api/utils.go | 6 +++++- main.go | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/api/core.go b/internal/api/core.go index 50cdf7b2..4f54dd4a 100644 --- a/internal/api/core.go +++ b/internal/api/core.go @@ -77,7 +77,7 @@ type v1Provider struct { // NewV1API creates an httpapi.API that serves the Limes v1 API. // It also returns the VersionData for this API version which is needed for the // version advertisement on "GET /". -func NewV1API(cluster *core.Cluster, dbm *gorp.DbMap, tokenValidator gopherpolicy.Validator, timeNow func() time.Time, generateToken func() string) httpapi.API { +func NewV1API(cluster *core.Cluster, dbm *gorp.DbMap, tokenValidator gopherpolicy.Validator, timeNow func() time.Time, generateTransferToken func() string) httpapi.API { p := &v1Provider{Cluster: cluster, DB: dbm, tokenValidator: tokenValidator, timeNow: timeNow} p.VersionData = VersionData{ Status: "CURRENT", @@ -94,7 +94,7 @@ func NewV1API(cluster *core.Cluster, dbm *gorp.DbMap, tokenValidator gopherpolic }, }, } - p.generateTransferToken = generateToken + p.generateTransferToken = generateTransferToken return p } @@ -113,7 +113,7 @@ func NewTokenValidator(provider *gophercloud.ProviderClient, eo gophercloud.Endp return &tv, err } -func (p *v1Provider) OverrideGenerateToken(generateTransferToken func() string) *v1Provider { +func (p *v1Provider) OverrideGenerateTransferToken(generateTransferToken func() string) *v1Provider { p.generateTransferToken = generateTransferToken return p } diff --git a/internal/api/utils.go b/internal/api/utils.go index a1b3cb99..977c0d8d 100644 --- a/internal/api/utils.go +++ b/internal/api/utils.go @@ -47,7 +47,11 @@ func unwrapOrDefault[T any](value *T, defaultValue T) T { return *value } -func GenerateToken() string { +/* +Generates a token that is used to transfer a commitment from a source to a target project. +The token will be attached to the commitment that will be transferred and stored in the database until the transfer is concluded. +*/ +func GenerateTransferToken() string { tokenBytes := make([]byte, 24) _, err := rand.Read(tokenBytes) if err != nil { diff --git a/main.go b/main.go index 1d412d6d..0e3ac95b 100644 --- a/main.go +++ b/main.go @@ -214,7 +214,7 @@ func taskServe(cluster *core.Cluster, args []string, provider *gophercloud.Provi }) mux := http.NewServeMux() mux.Handle("/", httpapi.Compose( - api.NewV1API(cluster, dbm, tokenValidator, time.Now, api.GenerateToken), + api.NewV1API(cluster, dbm, tokenValidator, time.Now, api.GenerateTransferToken), pprofapi.API{IsAuthorized: pprofapi.IsRequestFromLocalhost}, httpapi.WithGlobalMiddleware(api.ForbidClusterIDHeader), httpapi.WithGlobalMiddleware(corsMiddleware.Handler), From c777d8d6347d27d2a994bfd22fcfc0601e74fb1e Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 15:10:03 +0100 Subject: [PATCH 31/42] Fix: start transfer parseTarget struct now uses explicit fields. --- internal/api/commitment.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 87a9addc..996579d3 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -475,8 +475,12 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ http.Error(w, "project not found.", http.StatusNotFound) return } + // TODO: eventually migrate this struct into go-api-declarations var parseTarget struct { - Request limesresources.Commitment `json:"commitment"` + Request struct { + Amount uint64 `json:"amount"` + TransferStatus limesresources.CommitmentTransferStatus `json:"transfer_status,omitempty"` + } `json:"commitment"` } if !RequireJSON(w, r, &parseTarget) { http.Error(w, "json not parsable.", http.StatusBadRequest) From abf3a0fdf96b07eed69a68b8d7bf1770c49f163e Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 15:26:29 +0100 Subject: [PATCH 32/42] Fix: Errors now returned as text. Tests: Negative tests now check the body content --- internal/api/commitment.go | 7 ++++--- internal/api/commitment_test.go | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 996579d3..9d131aee 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -489,12 +489,12 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ req := parseTarget.Request if req.TransferStatus != limesresources.CommitmentTransferStatusUnlisted && req.TransferStatus != limesresources.CommitmentTransferStatusPublic { - respondwith.JSON(w, http.StatusBadRequest, fmt.Sprintf("Invalid transfer_status code. Must be %s or %s.", limesresources.CommitmentTransferStatusUnlisted, limesresources.CommitmentTransferStatusPublic)) + http.Error(w, fmt.Sprintf("Invalid transfer_status code. Must be %s or %s.", limesresources.CommitmentTransferStatusUnlisted, limesresources.CommitmentTransferStatusPublic), http.StatusBadRequest) return } if req.Amount <= 0 { - respondwith.JSON(w, http.StatusBadRequest, "Delivered amount needs to be a positive value.") + http.Error(w, "Delivered amount needs to be a positive value.", http.StatusBadRequest) return } @@ -601,7 +601,8 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) } transferToken := r.URL.Query().Get("token") if transferToken == "" { - respondwith.ErrorText(w, errors.New("no transfer token provided")) + http.Error(w, "no transfer token provided", http.StatusBadRequest) + return } dbDomain := p.FindDomainFromRequest(w, r) if dbDomain == nil { diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index fabe8756..d8ac65c3 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -677,6 +677,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", ExpectStatus: http.StatusBadRequest, + ExpectBody: assert.StringData("Delivered amount needs to be a positive value.\n"), Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 0, "transfer_status": "public"}}, }.Check(t, s.Handler) } @@ -759,12 +760,14 @@ func Test_TransferCommitment(t *testing.T) { Method: http.MethodPost, Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1?token=wrongToken", ExpectStatus: http.StatusNotFound, + ExpectBody: assert.StringData("no matching commitment found\n"), }.Check(t, s.Handler) // No token provided assert.HTTPRequest{ Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/1", - ExpectStatus: http.StatusInternalServerError, + ExpectStatus: http.StatusBadRequest, + ExpectBody: assert.StringData("no transfer token provided\n"), }.Check(t, s.Handler) } From 7a233cc2ad6af433355835c1bfcc4ae20b36ea0b Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 15:35:18 +0100 Subject: [PATCH 33/42] Request with amount > commitment amount will be treated as an error --- internal/api/commitment.go | 11 +++++++++-- internal/api/commitment_test.go | 13 +++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 9d131aee..1bef6c95 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -494,7 +494,7 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ } if req.Amount <= 0 { - http.Error(w, "Delivered amount needs to be a positive value.", http.StatusBadRequest) + http.Error(w, "delivered amount needs to be a positive value.", http.StatusBadRequest) return } @@ -521,7 +521,14 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ } defer sqlext.RollbackUnlessCommitted(tx) transferToken := p.generateTransferToken() - if req.Amount >= dbCommitment.Amount { + + // Deny requests with a greater amount than the commitment. + if req.Amount > dbCommitment.Amount { + http.Error(w, "delivered amount exceeds the commitment amount.", http.StatusBadRequest) + return + } + + if req.Amount == dbCommitment.Amount { _, err = tx.Exec(updateCommitmentTransferState, req.TransferStatus, transferToken, dbCommitment.ID) if respondwith.ErrorText(w, err) { return diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index d8ac65c3..f2891971 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -672,14 +672,23 @@ func Test_StartCommitmentTransfer(t *testing.T) { Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 10, "transfer_status": "unlisted"}}, }.Check(t, s.Handler) - // Negative Test, Amount = 0. + // Negative Test, amount = 0. assert.HTTPRequest{ Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", ExpectStatus: http.StatusBadRequest, - ExpectBody: assert.StringData("Delivered amount needs to be a positive value.\n"), + ExpectBody: assert.StringData("delivered amount needs to be a positive value.\n"), Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 0, "transfer_status": "public"}}, }.Check(t, s.Handler) + + // Negative Test, delivered amount > commitment amount + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", + ExpectStatus: http.StatusBadRequest, + ExpectBody: assert.StringData("delivered amount exceeds the commitment amount.\n"), + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 11, "transfer_status": "public"}}, + }.Check(t, s.Handler) } func Test_TransferCommitment(t *testing.T) { From 92de2167b318bca1fe125e4c8ceb1ebc9cac2c9e Mon Sep 17 00:00:00 2001 From: Nevostrius <52136954+VoigtS@users.noreply.github.com> Date: Fri, 15 Mar 2024 15:39:30 +0100 Subject: [PATCH 34/42] Update internal/api/commitment.go Simplify commitment transfer. Utilize ORM. Co-authored-by: Stefan Majewsky --- internal/api/commitment.go | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 1bef6c95..152802d2 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -639,32 +639,14 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) return } - tx, err := p.DB.Begin() - if respondwith.ErrorText(w, err) { - return - } - defer sqlext.RollbackUnlessCommitted(tx) - - // update AZ_RESOURCE_ID of commitment - _, err = tx.Exec(updateCommitmentAZResourceID, targetResourceID, dbCommitment.ID) - if respondwith.ErrorText(w, err) { - return - } - - // reset transfer_status and transfer_token - _, err = tx.Exec(updateCommitmentTransferState, "", "", dbCommitment.ID) - if respondwith.ErrorText(w, err) { - return - } - - err = tx.Commit() + dbCommitment.TransferStatus = "" + dbCommitment.TransferToken = "" + dbCommitment.AZResourceID = targetResourceID + err := p.DB.Update(&dbCommitment) if respondwith.ErrorText(w, err) { return } - dbCommitment.TransferStatus = "" - dbCommitment.TransferToken = "" - var loc azResourceLocation err = p.DB.QueryRow(findProjectAZResourceLocationByIDQuery, dbCommitment.AZResourceID). Scan(&loc.ServiceType, &loc.ResourceName, &loc.AvailabilityZone) From 8af9563d1439613df69e93a9b9ea02161ad53430 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 15:43:01 +0100 Subject: [PATCH 35/42] Fix: SQL update now takes both return values into account --- internal/api/commitment.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 152802d2..49e310ae 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -641,8 +641,8 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) dbCommitment.TransferStatus = "" dbCommitment.TransferToken = "" - dbCommitment.AZResourceID = targetResourceID - err := p.DB.Update(&dbCommitment) + dbCommitment.AZResourceID = targetResourceID + _, err = p.DB.Update(&dbCommitment) if respondwith.ErrorText(w, err) { return } From 944f2e65152c992cf65038eafd5927328ae88641 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 15:45:41 +0100 Subject: [PATCH 36/42] Fix: remove unused sql update query --- internal/api/commitment.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 49e310ae..70a8e893 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -110,9 +110,6 @@ var ( updateCommitmentSuperseded = sqlext.SimplifyWhitespace(` UPDATE project_commitments SET superseded_at = $1 WHERE id = $2 `) - updateCommitmentAZResourceID = sqlext.SimplifyWhitespace(` - UPDATE project_commitments SET az_resource_id = $1 WHERE id = $2 - `) ) // GetProjectCommitments handles GET /v1/domains/:domain_id/projects/:project_id/commitments. From f49743e42fa8f3ce0f290e1dfcf365d893459d5b Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 16:36:28 +0100 Subject: [PATCH 37/42] Fix: Remove SQL update queries and replace them with proper ORM Update transfer commitment test with split commitment --- internal/api/commitment.go | 17 +++++------- internal/api/commitment_test.go | 49 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 70a8e893..38979dd2 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -104,12 +104,6 @@ var ( WHERE cs.type = $2 AND cr.name = $3 ) `) - updateCommitmentTransferState = sqlext.SimplifyWhitespace(` - UPDATE project_commitments SET transfer_status = $1, transfer_token = $2 WHERE id = $3 - `) - updateCommitmentSuperseded = sqlext.SimplifyWhitespace(` - UPDATE project_commitments SET superseded_at = $1 WHERE id = $2 - `) ) // GetProjectCommitments handles GET /v1/domains/:domain_id/projects/:project_id/commitments. @@ -526,7 +520,9 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ } if req.Amount == dbCommitment.Amount { - _, err = tx.Exec(updateCommitmentTransferState, req.TransferStatus, transferToken, dbCommitment.ID) + dbCommitment.TransferStatus = req.TransferStatus + dbCommitment.TransferToken = transferToken + _, err = tx.Update(&dbCommitment) if respondwith.ErrorText(w, err) { return } @@ -535,6 +531,8 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ transferAmount := req.Amount remainingAmount := dbCommitment.Amount - req.Amount transferCommitment := p.buildSplitCommitment(dbCommitment, transferAmount) + transferCommitment.TransferStatus = req.TransferStatus + transferCommitment.TransferToken = transferToken remainingCommitment := p.buildSplitCommitment(dbCommitment, remainingAmount) err = tx.Insert(&transferCommitment) if respondwith.ErrorText(w, err) { @@ -544,14 +542,13 @@ func (p *v1Provider) StartCommitmentTransfer(w http.ResponseWriter, r *http.Requ if respondwith.ErrorText(w, err) { return } - _, err = tx.Exec(updateCommitmentSuperseded, now, dbCommitment.ID) + dbCommitment.SupersededAt = &now + _, err = tx.Update(&dbCommitment) if respondwith.ErrorText(w, err) { return } dbCommitment = transferCommitment } - dbCommitment.TransferStatus = req.TransferStatus - dbCommitment.TransferToken = transferToken err = tx.Commit() if respondwith.ErrorText(w, err) { return diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index f2891971..b1776337 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -741,6 +741,38 @@ func Test_TransferCommitment(t *testing.T) { "expires_at": 3600, } + // Split commitment + resp3 := assert.JSONObject{ + "id": 2, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-two", + "amount": 9, + "unit": "B", + "duration": "1 hour", + "created_at": s.Clock.Now().Unix(), + "creator_uuid": "uuid-for-alice", + "creator_name": "alice@Default", + "confirmed_at": 0, + "expires_at": 3600, + "transfer_status": "unlisted", + "transfer_token": transferToken, + } + resp4 := assert.JSONObject{ + "id": 2, + "service_type": "first", + "resource_name": "capacity", + "availability_zone": "az-two", + "amount": 9, + "unit": "B", + "duration": "1 hour", + "created_at": s.Clock.Now().Unix(), + "creator_uuid": "uuid-for-alice", + "creator_name": "alice@Default", + "confirmed_at": 0, + "expires_at": 3600, + } + // Transfer Commitment to target AZ_RESOURCE_ID (SOURCE_ID=3 TARGET_ID=17) assert.HTTPRequest{ Method: http.MethodPost, @@ -749,6 +781,7 @@ func Test_TransferCommitment(t *testing.T) { ExpectStatus: http.StatusCreated, }.Check(t, s.Handler) + // Transfer full amount assert.HTTPRequest{ Method: "POST", Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/start-transfer", @@ -764,6 +797,22 @@ func Test_TransferCommitment(t *testing.T) { ExpectStatus: http.StatusAccepted, }.Check(t, s.Handler) + // Split and transfer commitment. + assert.HTTPRequest{ + Method: "POST", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/commitments/1/start-transfer", + ExpectStatus: http.StatusAccepted, + ExpectBody: assert.JSONObject{"commitment": resp3}, + Body: assert.JSONObject{"commitment": assert.JSONObject{"amount": 9, "transfer_status": "unlisted"}}, + }.Check(t, s.Handler) + + assert.HTTPRequest{ + Method: http.MethodPost, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/1?token=" + transferToken, + ExpectBody: assert.JSONObject{"commitment": resp4}, + ExpectStatus: http.StatusAccepted, + }.Check(t, s.Handler) + // wrong token assert.HTTPRequest{ Method: http.MethodPost, From 0cfc807798a60ce47d5d9d879223047b88f57b60 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Fri, 15 Mar 2024 16:45:39 +0100 Subject: [PATCH 38/42] Fix docstring quotes --- internal/api/utils.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/api/utils.go b/internal/api/utils.go index 977c0d8d..9f922690 100644 --- a/internal/api/utils.go +++ b/internal/api/utils.go @@ -47,10 +47,8 @@ func unwrapOrDefault[T any](value *T, defaultValue T) T { return *value } -/* -Generates a token that is used to transfer a commitment from a source to a target project. -The token will be attached to the commitment that will be transferred and stored in the database until the transfer is concluded. -*/ +// Generates a token that is used to transfer a commitment from a source to a target project. +// The token will be attached to the commitment that will be transferred and stored in the database until the transfer is concluded. func GenerateTransferToken() string { tokenBytes := make([]byte, 24) _, err := rand.Read(tokenBytes) From 2553e6cec8ca3aa45c015bcb4c313c7071038f14 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 19 Mar 2024 15:57:12 +0100 Subject: [PATCH 39/42] Fix: Transfer Commitment did not use service or resource identifier Caused: on multipyle services or resources the first entry was selected and not the values the source commitments holds --- internal/api/commitment.go | 16 ++++++++++------ internal/api/commitment_test.go | 24 ++++++++++++------------ 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 38979dd2..053f48a8 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -87,15 +87,19 @@ var ( SELECT * FROM project_commitments WHERE transfer_token = $1 `) findTargetAZResourceIDBySourceIDQuery = sqlext.SimplifyWhitespace(` + WITH source as ( + SELECT ps.type, pr.name, par.az + FROM project_az_resources as par + JOIN project_resources pr ON par.resource_id = pr.id + JOIN project_services ps ON pr.service_id = ps.id + WHERE par.id = $1 + ) SELECT par.id FROM project_az_resources as par JOIN project_resources pr ON par.resource_id = pr.id JOIN project_services ps ON pr.service_id = ps.id - WHERE ps.project_id = $1 AND az = ( - SELECT az - FROM project_az_resources - WHERE id = $2 - ) + JOIN source s ON ps.type = s.type AND pr.name = s.name AND par.az = s.az + WHERE ps.project_id = $2 `) forceImmediateCapacityScrapeQuery = sqlext.SimplifyWhitespace(` @@ -628,7 +632,7 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) // get target AZ_RESOURCE_ID var targetResourceID db.ProjectAZResourceID - err = p.DB.QueryRow(findTargetAZResourceIDBySourceIDQuery, targetProject.ID, dbCommitment.AZResourceID).Scan(&targetResourceID) + err = p.DB.QueryRow(findTargetAZResourceIDBySourceIDQuery, dbCommitment.AZResourceID, targetProject.ID).Scan(&targetResourceID) if respondwith.ErrorText(w, err) { return } diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index b1776337..ff83fdd3 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -62,11 +62,11 @@ const testCommitmentsYAMLWithoutMinConfirmDate = ` type: --test-generic resource_behavior: # the resources in "first" have commitments, the ones in "second" do not - - resource: first/.* + - resource: second/.* commitment_durations: ["1 hour", "2 hours"] - - resource: first/things + - resource: second/things commitment_is_az_aware: false - - resource: first/capacity + - resource: second/capacity commitment_is_az_aware: true ` @@ -575,7 +575,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { req1 := func(transferStatus string) assert.JSONObject { return assert.JSONObject{ "id": 1, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 10, @@ -587,7 +587,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { resp1 := assert.JSONObject{ "id": 1, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 10, @@ -620,7 +620,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { // TransferAmount < CommitmentAmount resp2 := assert.JSONObject{ "id": 2, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 9, @@ -648,7 +648,7 @@ func Test_StartCommitmentTransfer(t *testing.T) { var confirmBy = s.Clock.Now().Unix() req2 := assert.JSONObject{ "id": 4, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 10, @@ -701,7 +701,7 @@ func Test_TransferCommitment(t *testing.T) { var transferToken = test.GenerateDummyToken() req1 := assert.JSONObject{ "id": 1, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 10, @@ -711,7 +711,7 @@ func Test_TransferCommitment(t *testing.T) { resp1 := assert.JSONObject{ "id": 1, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 10, @@ -728,7 +728,7 @@ func Test_TransferCommitment(t *testing.T) { resp2 := assert.JSONObject{ "id": 1, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 10, @@ -744,7 +744,7 @@ func Test_TransferCommitment(t *testing.T) { // Split commitment resp3 := assert.JSONObject{ "id": 2, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 9, @@ -760,7 +760,7 @@ func Test_TransferCommitment(t *testing.T) { } resp4 := assert.JSONObject{ "id": 2, - "service_type": "first", + "service_type": "second", "resource_name": "capacity", "availability_zone": "az-two", "amount": 9, From fa512f6f389f5066b5e26bfa8a34a9a6704d48a8 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 19 Mar 2024 16:19:06 +0100 Subject: [PATCH 40/42] Transfer commitment: no uses commitment ID and token to get the source commitment --- internal/api/commitment.go | 9 +++++++-- internal/api/commitment_test.go | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 053f48a8..9115d2b0 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -84,7 +84,7 @@ var ( WHERE par.id = $1 `) getCommitmentWithMatchingTransferTokenQuery = sqlext.SimplifyWhitespace(` - SELECT * FROM project_commitments WHERE transfer_token = $1 + SELECT * FROM project_commitments WHERE id = $1 AND transfer_token = $2 `) findTargetAZResourceIDBySourceIDQuery = sqlext.SimplifyWhitespace(` WITH source as ( @@ -609,6 +609,11 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) http.Error(w, "no transfer token provided", http.StatusBadRequest) return } + commitmentID := mux.Vars(r)["id"] + if commitmentID == "" { + http.Error(w, "no transfer token provided", http.StatusBadRequest) + return + } dbDomain := p.FindDomainFromRequest(w, r) if dbDomain == nil { http.Error(w, "domain not found.", http.StatusNotFound) @@ -622,7 +627,7 @@ func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) // find commitment by transfer_token var dbCommitment db.ProjectCommitment - err := p.DB.SelectOne(&dbCommitment, getCommitmentWithMatchingTransferTokenQuery, transferToken) + err := p.DB.SelectOne(&dbCommitment, getCommitmentWithMatchingTransferTokenQuery, commitmentID, transferToken) if errors.Is(err, sql.ErrNoRows) { http.Error(w, "no matching commitment found", http.StatusNotFound) return diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index ff83fdd3..385bbfcf 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -808,7 +808,7 @@ func Test_TransferCommitment(t *testing.T) { assert.HTTPRequest{ Method: http.MethodPost, - Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/1?token=" + transferToken, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/2?token=" + transferToken, ExpectBody: assert.JSONObject{"commitment": resp4}, ExpectStatus: http.StatusAccepted, }.Check(t, s.Handler) From fc307ae3bd99c9aeafe2d9ff34854c504b2026f2 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 19 Mar 2024 16:40:01 +0100 Subject: [PATCH 41/42] Transfer commitment: now uses transfer token in header - not as queryString anymore --- internal/api/commitment.go | 4 ++-- internal/api/commitment_test.go | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/api/commitment.go b/internal/api/commitment.go index 9115d2b0..cb682a92 100644 --- a/internal/api/commitment.go +++ b/internal/api/commitment.go @@ -598,13 +598,13 @@ func (p *v1Provider) buildSplitCommitment(dbCommitment db.ProjectCommitment, amo // TransferCommitment handles POST /v1/domains/{domain_id}/projects/{project_id}/transfer-commitment/{id}?token={token} func (p *v1Provider) TransferCommitment(w http.ResponseWriter, r *http.Request) { - httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/transfer-commitment/:id?token=:token") + httpapi.IdentifyEndpoint(r, "/v1/domains/:id/projects/:id/transfer-commitment/:id") token := p.CheckToken(r) if !token.Require(w, "project:edit") { http.Error(w, "insufficient access rights.", http.StatusForbidden) return } - transferToken := r.URL.Query().Get("token") + transferToken := r.Header.Get("Transfer-Token") if transferToken == "" { http.Error(w, "no transfer token provided", http.StatusBadRequest) return diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go index 385bbfcf..f2b555b4 100644 --- a/internal/api/commitment_test.go +++ b/internal/api/commitment_test.go @@ -792,7 +792,8 @@ func Test_TransferCommitment(t *testing.T) { assert.HTTPRequest{ Method: http.MethodPost, - Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1?token=" + transferToken, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1", + Header: map[string]string{"Transfer-Token": transferToken}, ExpectBody: assert.JSONObject{"commitment": resp2}, ExpectStatus: http.StatusAccepted, }.Check(t, s.Handler) @@ -808,7 +809,8 @@ func Test_TransferCommitment(t *testing.T) { assert.HTTPRequest{ Method: http.MethodPost, - Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/2?token=" + transferToken, + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/transfer-commitment/2", + Header: map[string]string{"Transfer-Token": transferToken}, ExpectBody: assert.JSONObject{"commitment": resp4}, ExpectStatus: http.StatusAccepted, }.Check(t, s.Handler) @@ -816,7 +818,8 @@ func Test_TransferCommitment(t *testing.T) { // wrong token assert.HTTPRequest{ Method: http.MethodPost, - Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1?token=wrongToken", + Path: "/v1/domains/uuid-for-germany/projects/uuid-for-dresden/transfer-commitment/1", + Header: map[string]string{"Transfer-Token": "wrongToken"}, ExpectStatus: http.StatusNotFound, ExpectBody: assert.StringData("no matching commitment found\n"), }.Check(t, s.Handler) From 77651027e13b886a7f637e20a08b39af04d50f55 Mon Sep 17 00:00:00 2001 From: VoigtS Date: Tue, 19 Mar 2024 16:47:48 +0100 Subject: [PATCH 42/42] Update transfer commitment documentation Respects the movement of the transfer token as queryString into the header --- docs/users/api-spec-resources.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md index 878ffaa9..b78aa396 100644 --- a/docs/users/api-spec-resources.md +++ b/docs/users/api-spec-resources.md @@ -773,9 +773,11 @@ The response is a JSON of the commitment including the following fields that ide } } ``` -### POST /v1/domains/:id/projects/:id/transfer-commitment/:id?token=:token +### POST /v1/domains/:id/projects/:id/transfer-commitment/:id Transfers the commitment from a source project to a target project. Requires a project-admin token. +Requires a transfer token in the request header: +`Transfer-Token: [value]`. This endpoint receives the target project ID, but the commitment ID from the source project. Requires a generated token from the API: `/v1/domains/:id/projects/:id/commitments/:id/start-transfer`. On success the API clears the `transfer_token` and `transfer_status` from the commitment.