From 2033ec28d7ae5a7ebcd4db28e219573015ca6de7 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 27 Mar 2026 23:48:08 -0700 Subject: [PATCH] s3api: add UnmarshalXML for Expiration, Transition, ExpireDeleteMarker Add UnmarshalXML methods that set the internal 'set' flag during XML parsing. Previously these flags were only set programmatically, causing XML round-trip to drop elements. This ensures lifecycle configurations stored as XML survive unmarshal/marshal cycles correctly. Add comprehensive XML round-trip tests for all lifecycle rule types including NoncurrentVersionExpiration, AbortIncompleteMultipartUpload, Filter with Tag/And/size constraints, and a complete Terraform-style lifecycle configuration. --- weed/s3api/s3api_policy.go | 31 +++++ weed/s3api/s3api_policy_test.go | 231 ++++++++++++++++++++++++++++++++ 2 files changed, 262 insertions(+) create mode 100644 weed/s3api/s3api_policy_test.go diff --git a/weed/s3api/s3api_policy.go b/weed/s3api/s3api_policy.go index 20745381e..cb715cba9 100644 --- a/weed/s3api/s3api_policy.go +++ b/weed/s3api/s3api_policy.go @@ -184,6 +184,17 @@ func (e Expiration) MarshalXML(enc *xml.Encoder, startElement xml.StartElement) return enc.EncodeElement(expirationWrapper(e), startElement) } +func (e *Expiration) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + type wrapper Expiration + var w wrapper + if err := d.DecodeElement(&w, &start); err != nil { + return err + } + *e = Expiration(w) + e.set = true + return nil +} + // ExpireDeleteMarker represents value of ExpiredObjectDeleteMarker field in Expiration XML element. type ExpireDeleteMarker struct { val bool @@ -198,6 +209,15 @@ func (b ExpireDeleteMarker) MarshalXML(e *xml.Encoder, startElement xml.StartEle return e.EncodeElement(b.val, startElement) } +func (b *ExpireDeleteMarker) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + var v bool + if err := d.DecodeElement(&v, &start); err != nil { + return err + } + *b = ExpireDeleteMarker{val: v, set: true} + return nil +} + // ExpirationDate is a embedded type containing time.Time to unmarshal // Date in Expiration type ExpirationDate struct { @@ -232,6 +252,17 @@ func (t Transition) MarshalXML(enc *xml.Encoder, start xml.StartElement) error { return enc.EncodeElement(transitionWrapper(t), start) } +func (t *Transition) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + type wrapper Transition + var w wrapper + if err := d.DecodeElement(&w, &start); err != nil { + return err + } + *t = Transition(w) + t.set = true + return nil +} + // TransitionDays is a type alias to unmarshal Days in Transition type TransitionDays int diff --git a/weed/s3api/s3api_policy_test.go b/weed/s3api/s3api_policy_test.go new file mode 100644 index 000000000..b14b4f824 --- /dev/null +++ b/weed/s3api/s3api_policy_test.go @@ -0,0 +1,231 @@ +package s3api + +import ( + "encoding/xml" + "strings" + "testing" +) + +func TestLifecycleXMLRoundTrip_NoncurrentVersionExpiration(t *testing.T) { + input := ` + + expire-noncurrent + Enabled + + + 30 + 2 + + +` + + var lc Lifecycle + if err := xml.Unmarshal([]byte(input), &lc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + if len(lc.Rules) != 1 { + t.Fatalf("expected 1 rule, got %d", len(lc.Rules)) + } + rule := lc.Rules[0] + if rule.ID != "expire-noncurrent" { + t.Errorf("expected ID 'expire-noncurrent', got %q", rule.ID) + } + if rule.NoncurrentVersionExpiration.NoncurrentDays != 30 { + t.Errorf("expected NoncurrentDays=30, got %d", rule.NoncurrentVersionExpiration.NoncurrentDays) + } + if rule.NoncurrentVersionExpiration.NewerNoncurrentVersions != 2 { + t.Errorf("expected NewerNoncurrentVersions=2, got %d", rule.NoncurrentVersionExpiration.NewerNoncurrentVersions) + } + + // Re-marshal and verify it round-trips. + out, err := xml.Marshal(lc) + if err != nil { + t.Fatalf("marshal: %v", err) + } + s := string(out) + if !strings.Contains(s, "30") { + t.Errorf("marshaled XML missing NoncurrentDays: %s", s) + } + if !strings.Contains(s, "2") { + t.Errorf("marshaled XML missing NewerNoncurrentVersions: %s", s) + } +} + +func TestLifecycleXMLRoundTrip_AbortIncompleteMultipartUpload(t *testing.T) { + input := ` + + abort-mpu + Enabled + + + 7 + + +` + + var lc Lifecycle + if err := xml.Unmarshal([]byte(input), &lc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + rule := lc.Rules[0] + if rule.AbortIncompleteMultipartUpload.DaysAfterInitiation != 7 { + t.Errorf("expected DaysAfterInitiation=7, got %d", rule.AbortIncompleteMultipartUpload.DaysAfterInitiation) + } + + out, err := xml.Marshal(lc) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if !strings.Contains(string(out), "7") { + t.Errorf("marshaled XML missing DaysAfterInitiation: %s", string(out)) + } +} + +func TestLifecycleXMLRoundTrip_FilterWithTag(t *testing.T) { + input := ` + + tag-filter + Enabled + + envdev + + 7 + +` + + var lc Lifecycle + if err := xml.Unmarshal([]byte(input), &lc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + rule := lc.Rules[0] + if !rule.Filter.tagSet { + t.Error("expected Filter.tagSet to be true") + } + if rule.Filter.Tag.Key != "env" || rule.Filter.Tag.Value != "dev" { + t.Errorf("expected Tag{env:dev}, got Tag{%s:%s}", rule.Filter.Tag.Key, rule.Filter.Tag.Value) + } +} + +func TestLifecycleXMLRoundTrip_FilterWithAnd(t *testing.T) { + input := ` + + and-filter + Enabled + + + logs/ + envdev + tierhot + 1024 + 1048576 + + + 7 + +` + + var lc Lifecycle + if err := xml.Unmarshal([]byte(input), &lc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + rule := lc.Rules[0] + if !rule.Filter.andSet { + t.Error("expected Filter.andSet to be true") + } + if rule.Filter.And.Prefix.String() != "logs/" { + t.Errorf("expected And.Prefix='logs/', got %q", rule.Filter.And.Prefix.String()) + } + if len(rule.Filter.And.Tags) != 2 { + t.Fatalf("expected 2 And tags, got %d", len(rule.Filter.And.Tags)) + } + if rule.Filter.And.ObjectSizeGreaterThan != 1024 { + t.Errorf("expected ObjectSizeGreaterThan=1024, got %d", rule.Filter.And.ObjectSizeGreaterThan) + } + if rule.Filter.And.ObjectSizeLessThan != 1048576 { + t.Errorf("expected ObjectSizeLessThan=1048576, got %d", rule.Filter.And.ObjectSizeLessThan) + } +} + +func TestLifecycleXMLRoundTrip_FilterWithSizeOnly(t *testing.T) { + input := ` + + size-filter + Enabled + + 512 + 10485760 + + 30 + +` + + var lc Lifecycle + if err := xml.Unmarshal([]byte(input), &lc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + rule := lc.Rules[0] + if rule.Filter.ObjectSizeGreaterThan != 512 { + t.Errorf("expected ObjectSizeGreaterThan=512, got %d", rule.Filter.ObjectSizeGreaterThan) + } + if rule.Filter.ObjectSizeLessThan != 10485760 { + t.Errorf("expected ObjectSizeLessThan=10485760, got %d", rule.Filter.ObjectSizeLessThan) + } +} + +func TestLifecycleXMLRoundTrip_CompleteRule(t *testing.T) { + // A complete lifecycle config similar to what Terraform sends. + input := ` + + rotation + + Enabled + 30 + + 1 + + + 1 + + +` + + var lc Lifecycle + if err := xml.Unmarshal([]byte(input), &lc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + rule := lc.Rules[0] + if rule.ID != "rotation" { + t.Errorf("expected ID 'rotation', got %q", rule.ID) + } + if rule.Expiration.Days != 30 { + t.Errorf("expected Expiration.Days=30, got %d", rule.Expiration.Days) + } + if rule.NoncurrentVersionExpiration.NoncurrentDays != 1 { + t.Errorf("expected NoncurrentDays=1, got %d", rule.NoncurrentVersionExpiration.NoncurrentDays) + } + if rule.AbortIncompleteMultipartUpload.DaysAfterInitiation != 1 { + t.Errorf("expected DaysAfterInitiation=1, got %d", rule.AbortIncompleteMultipartUpload.DaysAfterInitiation) + } + + // Re-marshal and verify all fields survive. + out, err := xml.Marshal(lc) + if err != nil { + t.Fatalf("marshal: %v", err) + } + s := string(out) + for _, expected := range []string{ + "30", + "1", + "1", + } { + if !strings.Contains(s, expected) { + t.Errorf("marshaled XML missing %q: %s", expected, s) + } + } +}