Add Duration Extension API (#576)

* Add scaffold to extend commitment duration depending on its config * Implement functionality to scuffold * Extension: use created_at date as basis, not now() * Align unit test to check for creation_date specifically * Rename API from extend to update duration * Adjust duration extension check to use direct date methods * Add documentation * Remove compareTo helper method replace it with a string compare * Fix leftovers wich mentioned extend-duration * Update internal/api/commitment.go Change provided duration check to a slice contain instead of checking their string representations. Co-authored-by: Stefan Majewsky <[email protected]> * Fix message attributes wich were undefined due to last commits changes * Omit explicit db transaction due to having only 1 operation on the database * Adjust error message and return state on forbidden duration conversion * Reject expired or superseded commitments * Fix unit test clock on last case * Replace wording in test comments replace date with the proper word duration --------- Co-authored-by: Stefan Majewsky <[email protected]>
sapcc · Oct 10, 2024 · f73791d · f73791d
1 parent 9ea9d1f
commit f73791d
Show file tree

Hide file tree

Showing 4 changed files with 252 additions and 1 deletion.
diff --git a/docs/users/api-spec-resources.md b/docs/users/api-spec-resources.md
@@ -628,6 +628,18 @@ Returns 202 (Accepted) on success, and and returns the converted commitment as a
 
 In this example, a commitment for 1 unit of the original resource can be converted into a commitment for 2 units of the target resource.
 
+### POST "/v1/domains/:domain_id/projects/:project_id/commitments/:commitment_id/update-duration"
+
+Change the duration of a commitment to a supported alternative.
+Requires a request body like:
+```json
+{
+	"duration": "3 years"
+}
+```
+
+Returns 200 (OK) on Success, and returns the updated 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).

diff --git a/internal/api/commitment.go b/internal/api/commitment.go
@@ -1045,3 +1045,95 @@ func (p *v1Provider) getCommitmentConversionRate(source, target core.ResourceBeh
 	toAmount = source.CommitmentConversion.Weight / divisor
 	return fromAmount, toAmount
 }
+
+// ExtendCommitmentDuration handles POST /v1/domains/{domain_id}/projects/{project_id}/commitments/{commitment_id}/update-duration
+func (p *v1Provider) UpdateCommitmentDuration(w http.ResponseWriter, r *http.Request) {
+	httpapi.IdentifyEndpoint(r, "/v1/domains/:domain_id/projects/:project_id/commitments/:commitment_id/update-duration")
+	token := p.CheckToken(r)
+	if !token.Require(w, "project:edit") {
+		return
+	}
+	commitmentID := mux.Vars(r)["commitment_id"]
+	if commitmentID == "" {
+		http.Error(w, "no transfer token provided", http.StatusBadRequest)
+		return
+	}
+	dbDomain := p.FindDomainFromRequest(w, r)
+	if dbDomain == nil {
+		return
+	}
+	dbProject := p.FindProjectFromRequest(w, r, dbDomain)
+	if dbProject == nil {
+		return
+	}
+	var Request struct {
+		Duration limesresources.CommitmentDuration `json:"duration"`
+	}
+	req := Request
+	if !RequireJSON(w, r, &req) {
+		return
+	}
+
+	var dbCommitment db.ProjectCommitment
+	err := p.DB.SelectOne(&dbCommitment, findProjectCommitmentByIDQuery, commitmentID, dbProject.ID)
+	if errors.Is(err, sql.ErrNoRows) {
+		http.Error(w, "no such commitment", http.StatusNotFound)
+		return
+	} else if respondwith.ErrorText(w, err) {
+		return
+	}
+
+	now := p.timeNow()
+	if dbCommitment.ExpiresAt.Before(now) || dbCommitment.ExpiresAt.Equal(now) {
+		http.Error(w, "unable to process expired commitment", http.StatusForbidden)
+		return
+	}
+
+	if dbCommitment.State == db.CommitmentStateSuperseded {
+		msg := fmt.Sprintf("unable to operate on commitment with a state of %s", dbCommitment.State)
+		http.Error(w, msg, http.StatusForbidden)
+		return
+	}
+
+	var loc datamodel.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
+	}
+	behavior := p.Cluster.BehaviorForResource(loc.ServiceType, loc.ResourceName)
+	if !slices.Contains(behavior.CommitmentDurations, req.Duration) {
+		msg := fmt.Sprintf("provided duration: %s does not match the config %v", req.Duration, behavior.CommitmentDurations)
+		http.Error(w, msg, http.StatusUnprocessableEntity)
+		return
+	}
+
+	newExpiresAt := req.Duration.AddTo(unwrapOrDefault(dbCommitment.ConfirmBy, dbCommitment.CreatedAt))
+	if newExpiresAt.Before(dbCommitment.ExpiresAt) {
+		msg := fmt.Sprintf("duration change from %s to %s forbidden", dbCommitment.Duration, req.Duration)
+		http.Error(w, msg, http.StatusForbidden)
+		return
+	}
+
+	dbCommitment.Duration = req.Duration
+	dbCommitment.ExpiresAt = newExpiresAt
+	_, err = p.DB.Update(&dbCommitment)
+	if respondwith.ErrorText(w, err) {
+		return
+	}
+
+	c := p.convertCommitmentToDisplayForm(dbCommitment, loc, token)
+	logAndPublishEvent(p.timeNow(), r, token, http.StatusAccepted, commitmentEventTarget{
+		DomainID:    dbDomain.UUID,
+		DomainName:  dbDomain.Name,
+		ProjectID:   dbProject.UUID,
+		ProjectName: dbProject.Name,
+		Commitments: []limesresources.Commitment{c},
+	})
+
+	respondwith.JSON(w, http.StatusOK, map[string]any{"commitment": c})
+}
diff --git a/internal/api/commitment_test.go b/internal/api/commitment_test.go
@@ -64,7 +64,7 @@ const testCommitmentsYAMLWithoutMinConfirmDate = `
 	resource_behavior:
 		# the resources in "first" have commitments, the ones in "second" do not
 		- resource: second/.*
-			commitment_durations: ["1 hour", "2 hours"]
+			commitment_durations: ["1 hour", "2 hours", "3 hours"]
 		- resource: second/things
 			commitment_is_az_aware: false
 		- resource: second/capacity
@@ -1325,3 +1325,149 @@ func Test_ConvertCommitments(t *testing.T) {
 		ExpectStatus: http.StatusAccepted,
 	}.Check(t, s.Handler)
 }
+
+func Test_UpdateCommitmentDuration(t *testing.T) {
+	s := test.NewSetup(t,
+		test.WithDBFixtureFile("fixtures/start-data-commitments.sql"),
+		test.WithConfig(testCommitmentsYAMLWithoutMinConfirmDate),
+		test.WithAPIHandler(NewV1API),
+	)
+
+	// Positive: confirmed commitment
+	assert.HTTPRequest{
+		Method: http.MethodPost,
+		Path:   "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/new",
+		Body: assert.JSONObject{
+			"commitment": assert.JSONObject{
+				"service_type":      "second",
+				"resource_name":     "capacity",
+				"availability_zone": "az-one",
+				"amount":            10,
+				"duration":          "2 hours",
+			},
+		},
+		ExpectStatus: http.StatusCreated,
+	}.Check(t, s.Handler)
+
+	// Fast forward by 1 hour. Creation_time = 0; Now = 1; (Expire = Creation_time + 2 hours)
+	s.Clock.StepBy(1 * time.Hour)
+
+	assert.HTTPRequest{
+		Method: http.MethodPost,
+		Path:   "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/update-duration",
+		Body:   assert.JSONObject{"duration": "3 hours"},
+		ExpectBody: assert.JSONObject{"commitment": assert.JSONObject{
+			"id":                1,
+			"service_type":      "second",
+			"resource_name":     "capacity",
+			"availability_zone": "az-one",
+			"amount":            10,
+			"unit":              "B",
+			"duration":          "3 hours",
+			"created_at":        s.Clock.Now().Add(-1 * time.Hour).Unix(),
+			"creator_uuid":      "uuid-for-alice",
+			"creator_name":      "alice@Default",
+			"can_be_deleted":    true,
+			"confirmed_at":      s.Clock.Now().Add(-1 * time.Hour).Unix(),
+			"expires_at":        s.Clock.Now().Add(2 * time.Hour).Unix(),
+		}},
+		ExpectStatus: http.StatusOK,
+	}.Check(t, s.Handler)
+
+	// Positive: Pending commitment
+	assert.HTTPRequest{
+		Method: http.MethodPost,
+		Path:   "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/new",
+		Body: assert.JSONObject{
+			"commitment": assert.JSONObject{
+				"service_type":      "second",
+				"resource_name":     "capacity",
+				"availability_zone": "az-one",
+				"amount":            10,
+				"confirm_by":        s.Clock.Now().Add(1 * day).Unix(),
+				"duration":          "1 hours",
+			},
+		},
+		ExpectStatus: http.StatusCreated,
+	}.Check(t, s.Handler)
+
+	assert.HTTPRequest{
+		Method: http.MethodPost,
+		Path:   "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/2/update-duration",
+		Body:   assert.JSONObject{"duration": "3 hours"},
+		ExpectBody: assert.JSONObject{"commitment": assert.JSONObject{
+			"id":                2,
+			"service_type":      "second",
+			"resource_name":     "capacity",
+			"availability_zone": "az-one",
+			"amount":            10,
+			"unit":              "B",
+			"duration":          "3 hours",
+			"created_at":        s.Clock.Now().Unix(),
+			"creator_uuid":      "uuid-for-alice",
+			"creator_name":      "alice@Default",
+			"can_be_deleted":    true,
+			"confirm_by":        s.Clock.Now().Add(1 * day).Unix(),
+			"expires_at":        s.Clock.Now().Add(3*time.Hour + 1*day).Unix(),
+		}},
+		ExpectStatus: http.StatusOK,
+	}.Check(t, s.Handler)
+
+	// Negative: Provided duration is invalid
+	assert.HTTPRequest{
+		Method:       http.MethodPost,
+		Path:         "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/update-duration",
+		Body:         assert.JSONObject{"duration": "99 hours"},
+		ExpectBody:   assert.StringData("provided duration: 99 hours does not match the config [1 hour 2 hours 3 hours]\n"),
+		ExpectStatus: http.StatusUnprocessableEntity,
+	}.Check(t, s.Handler)
+
+	// Negative: Provided duration < Commitment Duration
+	assert.HTTPRequest{
+		Method:       http.MethodPost,
+		Path:         "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/1/update-duration",
+		Body:         assert.JSONObject{"duration": "1 hour"},
+		ExpectBody:   assert.StringData("duration change from 3 hours to 1 hour forbidden\n"),
+		ExpectStatus: http.StatusForbidden,
+	}.Check(t, s.Handler)
+
+	// Negative: Expired commitment.
+	s.Clock.StepBy(-1 * time.Hour)
+	assert.HTTPRequest{
+		Method: http.MethodPost,
+		Path:   "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/new",
+		Body: assert.JSONObject{
+			"commitment": assert.JSONObject{
+				"service_type":      "second",
+				"resource_name":     "capacity",
+				"availability_zone": "az-one",
+				"amount":            10,
+				"duration":          "1 hours",
+			},
+		},
+		ExpectStatus: http.StatusCreated,
+	}.Check(t, s.Handler)
+
+	s.Clock.StepBy(1 * time.Hour)
+	assert.HTTPRequest{
+		Method:       http.MethodPost,
+		Path:         "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/3/update-duration",
+		Body:         assert.JSONObject{"duration": "2 hours"},
+		ExpectBody:   assert.StringData("unable to process expired commitment\n"),
+		ExpectStatus: http.StatusForbidden,
+	}.Check(t, s.Handler)
+
+	// Negative: Superseded commitment
+	s.Clock.StepBy(-1 * time.Hour)
+	_, err := s.DB.Exec("UPDATE project_commitments SET state='superseded' where ID = 3")
+	if err != nil {
+		t.Fatal(err)
+	}
+	assert.HTTPRequest{
+		Method:       http.MethodPost,
+		Path:         "/v1/domains/uuid-for-germany/projects/uuid-for-berlin/commitments/3/update-duration",
+		Body:         assert.JSONObject{"duration": "2 hours"},
+		ExpectBody:   assert.StringData("unable to operate on commitment with a state of superseded\n"),
+		ExpectStatus: http.StatusForbidden,
+	}.Check(t, s.Handler)
+}
diff --git a/internal/api/core.go b/internal/api/core.go
@@ -167,6 +167,7 @@ func (p *v1Provider) AddTo(r *mux.Router) {
 	r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/transfer-commitment/{id}").HandlerFunc(p.TransferCommitment)
 	r.Methods("GET").Path("/v1/commitment-conversion/{service_type}/{resource_name}").HandlerFunc(p.GetCommitmentConversions)
 	r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{commitment_id}/convert").HandlerFunc(p.ConvertCommitment)
+	r.Methods("POST").Path("/v1/domains/{domain_id}/projects/{project_id}/commitments/{commitment_id}/update-duration").HandlerFunc(p.UpdateCommitmentDuration)
 }
 
 // RequireJSON will parse the request body into the given data structure, or