Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(storage): allow to use age=0 in OLM conditions #6204

Merged
merged 10 commits into from Aug 26, 2022
63 changes: 33 additions & 30 deletions storage/bucket.go
Expand Up @@ -610,7 +610,12 @@ const (
//
// All configured conditions must be met for the associated action to be taken.
type LifecycleCondition struct {
// AllObject is used to select all objects in a bucket by
tritone marked this conversation as resolved.
Show resolved Hide resolved
// setting AgeInDays to 0.
AllObjects bool

// AgeInDays is the age of the object in days.
// If you want to set AgeInDays to `0` use AllObjects set to `true`.
AgeInDays int64

// CreatedBefore is the time the object was created.
Expand All @@ -628,10 +633,12 @@ type LifecycleCondition struct {

// DaysSinceCustomTime is the days elapsed since the CustomTime date of the
// object. This condition can only be satisfied if CustomTime has been set.
// Note: Using `0` as the value will be ignored by the library and not sent to the API.
DaysSinceCustomTime int64

// DaysSinceNoncurrentTime is the days elapsed since the noncurrent timestamp
// of the object. This condition is relevant only for versioned objects.
// Note: Using `0` as the value will be ignored by the library and not sent to the API.
DaysSinceNoncurrentTime int64

// Liveness specifies the object's liveness. Relevant only for versioned objects
Expand Down Expand Up @@ -663,6 +670,7 @@ type LifecycleCondition struct {
// If the value is N, this condition is satisfied when there are at least N
// versions (including the live version) newer than this version of the
// object.
// Note: Using `0` as the value will be ignored by the library and not sent to the API.
NumNewerVersions int64
}

Expand Down Expand Up @@ -1421,19 +1429,6 @@ func toCORSFromProto(rc []*storagepb.Bucket_Cors) []CORS {
return out
}

// Used to handle breaking change in Autogen Storage client OLM Age field
// from int64 to *int64 gracefully in the manual client
// TODO(#6240): Method should be removed once breaking change is made and introduced to this client
func setAgeCondition(age int64, ageField interface{}) {
c := reflect.ValueOf(ageField).Elem()
switch c.Kind() {
case reflect.Int64:
c.SetInt(age)
case reflect.Ptr:
c.Set(reflect.ValueOf(&age))
}
}

func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle {
var rl raw.BucketLifecycle
if len(l.Rules) == 0 {
Expand All @@ -1455,7 +1450,15 @@ func toRawLifecycle(l Lifecycle) *raw.BucketLifecycle {
},
}

setAgeCondition(r.Condition.AgeInDays, &rr.Condition.Age)
// AllObjects takes precedent when both AllObjects and AgeInDays are set
// Rationale: If you've opted into using AllObjects, it makes sense that you
// understand the implications of how this option works with AgeInDays.
if r.Condition.AllObjects {
tritone marked this conversation as resolved.
Show resolved Hide resolved
rr.Condition.Age = googleapi.Int64(0)
rr.Condition.ForceSendFields = []string{"Age"}
} else if r.Condition.AgeInDays > 0 {
rr.Condition.Age = googleapi.Int64(r.Condition.AgeInDays)
}

switch r.Condition.Liveness {
case LiveAndArchived:
Expand Down Expand Up @@ -1504,6 +1507,11 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle {
},
}

// TODO(#6205): This may not be needed for gRPC
if r.Condition.AllObjects {
rr.Condition.AgeDays = proto.Int32(0)
}

switch r.Condition.Liveness {
case LiveAndArchived:
rr.Condition.IsLive = nil
Expand All @@ -1527,21 +1535,6 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle {
return &rl
}

// Used to handle breaking change in Autogen Storage client OLM Age field
// from int64 to *int64 gracefully in the manual client
// TODO(#6240): Method should be removed once breaking change is made and introduced to this client
func getAgeCondition(ageField interface{}) int64 {
v := reflect.ValueOf(ageField)
if v.Kind() == reflect.Int64 {
return v.Interface().(int64)
} else if v.Kind() == reflect.Ptr {
if val, ok := v.Interface().(*int64); ok {
return *val
}
}
return 0
}

func toLifecycle(rl *raw.BucketLifecycle) Lifecycle {
var l Lifecycle
if rl == nil {
Expand All @@ -1562,7 +1555,12 @@ func toLifecycle(rl *raw.BucketLifecycle) Lifecycle {
NumNewerVersions: rr.Condition.NumNewerVersions,
},
}
r.Condition.AgeInDays = getAgeCondition(rr.Condition.Age)
if rr.Condition.Age != nil {
r.Condition.AgeInDays = *rr.Condition.Age
if *rr.Condition.Age == 0 {
r.Condition.AllObjects = true
}
}
tritone marked this conversation as resolved.
Show resolved Hide resolved

if rr.Condition.IsLive == nil {
r.Condition.Liveness = LiveAndArchived
Expand Down Expand Up @@ -1608,6 +1606,11 @@ func toLifecycleFromProto(rl *storagepb.Bucket_Lifecycle) Lifecycle {
},
}

// TODO(#6205): This may not be needed for gRPC
tritone marked this conversation as resolved.
Show resolved Hide resolved
if rr.GetCondition().GetAgeDays() == 0 {
r.Condition.AllObjects = true
}

if rr.GetCondition().IsLive == nil {
r.Condition.Liveness = LiveAndArchived
} else if rr.GetCondition().GetIsLive() {
Expand Down
55 changes: 24 additions & 31 deletions storage/bucket_test.go
Expand Up @@ -25,9 +25,7 @@ import (
raw "google.golang.org/api/storage/v1"
)

// TODO(#6539): re-enable tests after breaking change is released and AgeInDays is a int64*
func TestBucketAttrsToRawBucket(t *testing.T) {
t.Skip("TestBucketAttrsToRawBucket skipped: https://github.com/googleapis/google-cloud-go/issues/6539")
t.Parallel()
attrs := &BucketAttrs{
Name: "name",
Expand Down Expand Up @@ -119,6 +117,13 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
Condition: LifecycleCondition{
AgeInDays: 20,
},
}, {
Action: LifecycleAction{
Type: DeleteAction,
},
Condition: LifecycleCondition{
AllObjects: true,
},
}},
},
}
Expand Down Expand Up @@ -163,7 +168,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
StorageClass: "NEARLINE",
},
Condition: &raw.BucketLifecycleRuleCondition{
//Age: 10,
Age: googleapi.Int64(10),
IsLive: googleapi.Bool(true),
CreatedBefore: "2017-01-02",
MatchesStorageClass: []string{"STANDARD"},
Expand Down Expand Up @@ -199,7 +204,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
Type: DeleteAction,
},
Condition: &raw.BucketLifecycleRuleCondition{
//Age: 10,
Age: googleapi.Int64(10),
MatchesPrefix: []string{"testPrefix"},
MatchesSuffix: []string{"testSuffix"},
NumNewerVersions: 3,
Expand All @@ -218,7 +223,16 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
Type: AbortIncompleteMPUAction,
},
Condition: &raw.BucketLifecycleRuleCondition{
//Age: 20,
Age: googleapi.Int64(20),
},
},
{
Action: &raw.BucketLifecycleRuleAction{
Type: DeleteAction,
},
Condition: &raw.BucketLifecycleRuleCondition{
Age: googleapi.Int64(0),
ForceSendFields: []string{"Age"},
},
},
},
Expand Down Expand Up @@ -350,9 +364,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
}
}

// TODO(#6539): re-enable tests after breaking change is released and AgeInDays is a int64*
func TestBucketAttrsToUpdateToRawBucket(t *testing.T) {
t.Skip("TestBucketAttrsToUpdateToRawBucket skipped: https://github.com/googleapis/google-cloud-go/issues/6539")
t.Parallel()
au := &BucketAttrsToUpdate{
VersioningEnabled: false,
Expand Down Expand Up @@ -408,12 +420,12 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) {
Lifecycle: &raw.BucketLifecycle{
Rule: []*raw.BucketLifecycleRule{
{
Action: &raw.BucketLifecycleRuleAction{Type: "Delete"},
//Condition: &raw.BucketLifecycleRuleCondition{Age: 30},
Action: &raw.BucketLifecycleRuleAction{Type: "Delete"},
Condition: &raw.BucketLifecycleRuleCondition{Age: googleapi.Int64(30)},
},
{
Action: &raw.BucketLifecycleRuleAction{Type: AbortIncompleteMPUAction},
//Condition: &raw.BucketLifecycleRuleCondition{Age: 13},
Action: &raw.BucketLifecycleRuleAction{Type: AbortIncompleteMPUAction},
Condition: &raw.BucketLifecycleRuleCondition{Age: googleapi.Int64(13)},
},
},
},
Expand Down Expand Up @@ -564,26 +576,7 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) {
}
}

func TestAgeConditionBackwardCompat(t *testing.T) {
var ti int64
var want int64 = 100
setAgeCondition(want, &ti)
if getAgeCondition(ti) != want {
t.Fatalf("got %v, want %v", getAgeCondition(ti), want)
}

var tp *int64
want = 10
setAgeCondition(want, &tp)
if getAgeCondition(tp) != want {
t.Fatalf("got %v, want %v", getAgeCondition(tp), want)
}

}

// TODO(#6539): re-enable tests after breaking change is released and AgeInDays is a int64*
func TestNewBucket(t *testing.T) {
t.Skip("TestNewBucket skipped: https://github.com/googleapis/google-cloud-go/issues/6539")
labels := map[string]string{"a": "b"}
matchClasses := []string{"STANDARD"}
aTime := time.Date(2017, 1, 2, 0, 0, 0, 0, time.UTC)
Expand All @@ -605,7 +598,7 @@ func TestNewBucket(t *testing.T) {
StorageClass: "NEARLINE",
},
Condition: &raw.BucketLifecycleRuleCondition{
// Age: 10,
Age: googleapi.Int64(10),
IsLive: googleapi.Bool(true),
CreatedBefore: "2017-01-02",
MatchesStorageClass: matchClasses,
Expand Down
11 changes: 11 additions & 0 deletions storage/integration_test.go
Expand Up @@ -309,6 +309,13 @@ func TestIntegration_BucketCreateDelete(t *testing.T) {
MatchesSuffix: []string{"testSuffix"},
NumNewerVersions: 3,
},
}, {
Action: LifecycleAction{
Type: DeleteAction,
},
Condition: LifecycleCondition{
AllObjects: true,
},
}},
}

Expand Down Expand Up @@ -442,6 +449,10 @@ func TestIntegration_BucketLifecycle(t *testing.T) {
Action: LifecycleAction{Type: AbortIncompleteMPUAction},
Condition: LifecycleCondition{AgeInDays: 30},
},
{
Action: LifecycleAction{Type: DeleteAction},
Condition: LifecycleCondition{AllObjects: true},
},
},
}

Expand Down