refactor(s3api): move Lifecycle XML structs to leaf package lifecycle_xml (#9360)

* refactor(s3api): move Lifecycle XML structs to leaf package lifecycle_xml

The structs S3 PutBucketLifecycleConfiguration parses and the canonical
conversion to s3lifecycle.Rule lived in package s3api, which transitively
imports weed/server (which imports weed/shell). Any caller outside
weed/s3api — the shell, the future lifecycle worker — that wanted to
parse a bucket's lifecycle XML hit an import cycle.

Moves:
  weed/s3api/s3api_policy.go              -> lifecycle_xml/types.go
  weed/s3api/s3api_lifecycle_canonical.go -> lifecycle_xml/canonical.go
  s3api_lifecycle_canonical_test.go       -> lifecycle_xml/canonical_test.go
  s3api_policy_test.go                    -> lifecycle_xml/round_trip_test.go

Renames the public RuleStatus type (was unexported ruleStatus) and adds
small accessor methods (Set/Val/AndSet/TagSet) for fields the s3api
handler needs to read across the package boundary. Adds NewPrefix and
NewExpirationDays constructors so the GET handler can build response
rules without poking at unexported fields. Adds a Tag struct local to
the package so it has zero internal seaweed deps. Adds a one-shot
ParseCanonical(xml []byte) helper for non-server callers.

s3api_policy.go was misnamed — its content is lifecycle XML, not S3
bucket policy. The new package name reflects the actual scope.

* test(s3api/lifecycle_xml): exercise public API in tests

- canonical_test.go's parseLifecycle helper went through xml.Unmarshal
  directly; route it through the package's exported Parse so tests
  validate the public entrypoint.
- round_trip_test.go asserted internal flags (rule.Filter.tagSet,
  rule.Filter.andSet, Transition.set, NoncurrentVersionTransition.set);
  switch to TagSet(), AndSet(), Set() — exercises the public contract
  that downstream callers (s3api handler, future shell command) rely on.
This commit is contained in:
Chris Lu
2026-05-07 18:54:06 -07:00
committed by GitHub
parent c567da7164
commit b7928637a0
5 changed files with 112 additions and 42 deletions
@@ -1,9 +1,32 @@
package s3api package lifecycle_xml
import ( import (
"bytes"
"encoding/xml"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle" "github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle"
) )
// Parse decodes a BucketLifecycleConfiguration XML body into the wire-form
// Lifecycle struct.
func Parse(xmlBytes []byte) (*Lifecycle, error) {
var lc Lifecycle
if err := xml.NewDecoder(bytes.NewReader(xmlBytes)).Decode(&lc); err != nil {
return nil, err
}
return &lc, nil
}
// ParseCanonical is the one-shot path most non-server callers want:
// raw XML in, []*s3lifecycle.Rule out.
func ParseCanonical(xmlBytes []byte) ([]*s3lifecycle.Rule, error) {
lc, err := Parse(xmlBytes)
if err != nil {
return nil, err
}
return LifecycleToCanonical(lc), nil
}
// LifecycleToCanonical flattens the XML-deserialized Lifecycle into the // LifecycleToCanonical flattens the XML-deserialized Lifecycle into the
// engine's flat Rule shape. The optional <Filter> element may contain // engine's flat Rule shape. The optional <Filter> element may contain
// <Prefix> | <Tag> | <And>, or be absent (in which case the older top-level // <Prefix> | <Tag> | <And>, or be absent (in which case the older top-level
@@ -1,20 +1,19 @@
package s3api package lifecycle_xml
import ( import (
"encoding/xml"
"reflect" "reflect"
"testing" "testing"
"time" "time"
) )
// parseLifecycle is a thin helper for tests; production code reads XML via the // parseLifecycle is a thin helper for tests; production code reads XML via
// regular bucket-config decoder, this shortcut keeps the test focused on the // the regular bucket-config decoder, this shortcut goes through the
// canonical conversion. // package's public Parse so the tests exercise the exported entrypoint.
func parseLifecycle(t *testing.T, xmlSrc string) *Lifecycle { func parseLifecycle(t *testing.T, xmlSrc string) *Lifecycle {
t.Helper() t.Helper()
lc := &Lifecycle{} lc, err := Parse([]byte(xmlSrc))
if err := xml.Unmarshal([]byte(xmlSrc), lc); err != nil { if err != nil {
t.Fatalf("unmarshal: %v", err) t.Fatalf("parse: %v", err)
} }
return lc return lc
} }
@@ -1,4 +1,4 @@
package s3api package lifecycle_xml
import ( import (
"encoding/xml" "encoding/xml"
@@ -101,8 +101,8 @@ func TestLifecycleXMLRoundTrip_FilterWithTag(t *testing.T) {
} }
rule := lc.Rules[0] rule := lc.Rules[0]
if !rule.Filter.tagSet { if !rule.Filter.TagSet() {
t.Error("expected Filter.tagSet to be true") t.Error("expected Filter.TagSet() to be true")
} }
if rule.Filter.Tag.Key != "env" || rule.Filter.Tag.Value != "dev" { 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) t.Errorf("expected Tag{env:dev}, got Tag{%s:%s}", rule.Filter.Tag.Key, rule.Filter.Tag.Value)
@@ -133,8 +133,8 @@ func TestLifecycleXMLRoundTrip_FilterWithAnd(t *testing.T) {
} }
rule := lc.Rules[0] rule := lc.Rules[0]
if !rule.Filter.andSet { if !rule.Filter.AndSet() {
t.Error("expected Filter.andSet to be true") t.Error("expected Filter.AndSet() to be true")
} }
if rule.Filter.And.Prefix.String() != "logs/" { if rule.Filter.And.Prefix.String() != "logs/" {
t.Errorf("expected And.Prefix='logs/', got %q", rule.Filter.And.Prefix.String()) t.Errorf("expected And.Prefix='logs/', got %q", rule.Filter.And.Prefix.String())
@@ -178,7 +178,7 @@ func TestLifecycleXMLRoundTrip_FilterWithSizeOnly(t *testing.T) {
} }
func TestLifecycleXML_TransitionSetFlag(t *testing.T) { func TestLifecycleXML_TransitionSetFlag(t *testing.T) {
// Verify that Transition.set is true after unmarshaling. // Verify that Transition.Set() is true after unmarshaling.
input := `<LifecycleConfiguration> input := `<LifecycleConfiguration>
<Rule> <Rule>
<ID>transition</ID> <ID>transition</ID>
@@ -195,8 +195,8 @@ func TestLifecycleXML_TransitionSetFlag(t *testing.T) {
if err := xml.Unmarshal([]byte(input), &lc); err != nil { if err := xml.Unmarshal([]byte(input), &lc); err != nil {
t.Fatalf("unmarshal: %v", err) t.Fatalf("unmarshal: %v", err)
} }
if !lc.Rules[0].Transition.set { if !lc.Rules[0].Transition.Set() {
t.Error("expected Transition.set=true after unmarshal") t.Error("expected Transition.Set()=true after unmarshal")
} }
} }
@@ -217,8 +217,8 @@ func TestLifecycleXML_NoncurrentVersionTransitionSetFlag(t *testing.T) {
if err := xml.Unmarshal([]byte(input), &lc); err != nil { if err := xml.Unmarshal([]byte(input), &lc); err != nil {
t.Fatalf("unmarshal: %v", err) t.Fatalf("unmarshal: %v", err)
} }
if !lc.Rules[0].NoncurrentVersionTransition.set { if !lc.Rules[0].NoncurrentVersionTransition.Set() {
t.Error("expected NoncurrentVersionTransition.set=true after unmarshal") t.Error("expected NoncurrentVersionTransition.Set()=true after unmarshal")
} }
} }
@@ -1,19 +1,36 @@
package s3api // Package lifecycle_xml is the XML wire-form for S3 BucketLifecycleConfiguration:
// the structs S3 PutBucketLifecycleConfiguration accepts and
// GetBucketLifecycleConfiguration returns. It lives outside weed/s3api so
// callers outside that package (shell, lifecycle worker) can parse and
// emit the same shape without pulling weed/s3api's full dependency graph
// — weed/s3api transitively imports weed/server, which imports weed/shell.
//
// Conversion to the engine-friendly canonical form
// (weed/s3api/s3lifecycle.Rule) lives next to the types here as
// LifecycleToCanonical.
package lifecycle_xml
import ( import (
"encoding/xml" "encoding/xml"
"time" "time"
) )
// Status represents lifecycle configuration status // RuleStatus represents lifecycle rule status.
type ruleStatus string type RuleStatus string
// Supported status types // Supported status types.
const ( const (
Enabled ruleStatus = "Enabled" Enabled RuleStatus = "Enabled"
Disabled ruleStatus = "Disabled" Disabled RuleStatus = "Disabled"
) )
// Tag is the lifecycle filter tag pair. Defined locally so this package
// has no dependency on weed/s3api.
type Tag struct {
Key string `xml:"Key"`
Value string `xml:"Value"`
}
// Lifecycle - Configuration for bucket lifecycle. // Lifecycle - Configuration for bucket lifecycle.
type Lifecycle struct { type Lifecycle struct {
XMLName xml.Name `xml:"LifecycleConfiguration"` XMLName xml.Name `xml:"LifecycleConfiguration"`
@@ -24,7 +41,7 @@ type Lifecycle struct {
type Rule struct { type Rule struct {
XMLName xml.Name `xml:"Rule"` XMLName xml.Name `xml:"Rule"`
ID string `xml:"ID,omitempty"` ID string `xml:"ID,omitempty"`
Status ruleStatus `xml:"Status"` Status RuleStatus `xml:"Status"`
Filter Filter `xml:"Filter,omitempty"` Filter Filter `xml:"Filter,omitempty"`
Prefix Prefix `xml:"Prefix,omitempty"` Prefix Prefix `xml:"Prefix,omitempty"`
Expiration Expiration `xml:"Expiration,omitempty"` Expiration Expiration `xml:"Expiration,omitempty"`
@@ -63,6 +80,36 @@ func (p Prefix) String() string {
return p.val return p.val
} }
// Set returns whether the Prefix carries a value (vs being absent).
func (p Prefix) Set() bool { return p.set }
// Val returns the prefix string. Empty when !Set.
func (p Prefix) Val() string { return p.val }
// NewPrefix builds a Prefix marked as set.
func NewPrefix(val string) Prefix { return Prefix{val: val, set: true} }
// NewExpirationDays builds an Expiration with Days set.
func NewExpirationDays(days int) Expiration { return Expiration{Days: days, set: true} }
// Set reports whether this Filter is present in the XML.
func (f Filter) Set() bool { return f.set }
// AndSet reports whether the Filter contains an <And> branch.
func (f Filter) AndSet() bool { return f.andSet }
// TagSet reports whether the Filter contains a single <Tag> branch.
func (f Filter) TagSet() bool { return f.tagSet }
// Set reports whether the Transition element was present.
func (t Transition) Set() bool { return t.set }
// Set reports whether the NoncurrentVersionTransition element was present.
func (n NoncurrentVersionTransition) Set() bool { return n.set }
// Set reports whether the Expiration element was present.
func (e Expiration) Set() bool { return e.set }
// MarshalXML encodes Prefix field into an XML form. // MarshalXML encodes Prefix field into an XML form.
func (p Prefix) MarshalXML(e *xml.Encoder, startElement xml.StartElement) error { func (p Prefix) MarshalXML(e *xml.Encoder, startElement xml.StartElement) error {
if !p.set { if !p.set {
+17 -16
View File
@@ -20,6 +20,7 @@ import (
"github.com/seaweedfs/seaweedfs/weed/s3api/s3bucket" "github.com/seaweedfs/seaweedfs/weed/s3api/s3bucket"
"github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/filer"
"github.com/seaweedfs/seaweedfs/weed/s3api/lifecycle_xml"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
stats_collect "github.com/seaweedfs/seaweedfs/weed/stats" stats_collect "github.com/seaweedfs/seaweedfs/weed/stats"
"github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/needle"
@@ -840,7 +841,7 @@ func (s3a *S3ApiServer) GetBucketLifecycleConfigurationHandler(w http.ResponseWr
return return
} }
response := Lifecycle{} response := lifecycle_xml.Lifecycle{}
// Sort locationPrefixes to ensure consistent ordering of lifecycle rules // Sort locationPrefixes to ensure consistent ordering of lifecycle rules
var locationPrefixes []string var locationPrefixes []string
for locationPrefix := range ttls { for locationPrefix := range ttls {
@@ -859,11 +860,11 @@ func (s3a *S3ApiServer) GetBucketLifecycleConfigurationHandler(w http.ResponseWr
if !found { if !found {
continue continue
} }
response.Rules = append(response.Rules, Rule{ response.Rules = append(response.Rules, lifecycle_xml.Rule{
ID: prefix, ID: prefix,
Status: Enabled, Status: lifecycle_xml.Enabled,
Prefix: Prefix{val: prefix, set: true}, Prefix: lifecycle_xml.NewPrefix(prefix),
Expiration: Expiration{Days: days, set: true}, Expiration: lifecycle_xml.NewExpirationDays(days),
}) })
} }
@@ -920,7 +921,7 @@ func (s3a *S3ApiServer) PutBucketLifecycleConfigurationHandler(w http.ResponseWr
return return
} }
lifeCycleConfig := Lifecycle{} lifeCycleConfig := lifecycle_xml.Lifecycle{}
if err := xmlDecoder(bytes.NewReader(lifecycleXML), &lifeCycleConfig, int64(len(lifecycleXML))); err != nil { if err := xmlDecoder(bytes.NewReader(lifecycleXML), &lifeCycleConfig, int64(len(lifecycleXML))); err != nil {
glog.Warningf("PutBucketLifecycleConfigurationHandler xml decode: %s", err) glog.Warningf("PutBucketLifecycleConfigurationHandler xml decode: %s", err)
s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML) s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML)
@@ -977,12 +978,12 @@ func (s3a *S3ApiServer) PutBucketLifecycleConfigurationHandler(w http.ResponseWr
bucketVersioning == s3_constants.VersioningSuspended bucketVersioning == s3_constants.VersioningSuspended
for _, rule := range lifeCycleConfig.Rules { for _, rule := range lifeCycleConfig.Rules {
if rule.Status != Enabled { if rule.Status != lifecycle_xml.Enabled {
continue continue
} }
// Reject Transition rules — they require storage class migration // Reject Transition rules — they require storage class migration
// infrastructure that does not exist yet. // infrastructure that does not exist yet.
if rule.Transition.set || rule.NoncurrentVersionTransition.set { if rule.Transition.Set() || rule.NoncurrentVersionTransition.Set() {
s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented)
return return
} }
@@ -993,12 +994,12 @@ func (s3a *S3ApiServer) PutBucketLifecycleConfigurationHandler(w http.ResponseWr
var rulePrefix string var rulePrefix string
switch { switch {
case rule.Filter.andSet: case rule.Filter.AndSet():
rulePrefix = rule.Filter.And.Prefix.val rulePrefix = rule.Filter.And.Prefix.Val()
case rule.Filter.Prefix.set: case rule.Filter.Prefix.Set():
rulePrefix = rule.Filter.Prefix.val rulePrefix = rule.Filter.Prefix.Val()
case rule.Prefix.set: case rule.Prefix.Set():
rulePrefix = rule.Prefix.val rulePrefix = rule.Prefix.Val()
} }
// Only create filer.conf TTL entries for simple Expiration.Days rules // Only create filer.conf TTL entries for simple Expiration.Days rules
@@ -1009,9 +1010,9 @@ func (s3a *S3ApiServer) PutBucketLifecycleConfigurationHandler(w http.ResponseWr
if rule.Expiration.Days == 0 { if rule.Expiration.Days == 0 {
continue continue
} }
hasTagOrSizeFilter := rule.Filter.tagSet || hasTagOrSizeFilter := rule.Filter.TagSet() ||
rule.Filter.ObjectSizeGreaterThan > 0 || rule.Filter.ObjectSizeLessThan > 0 || rule.Filter.ObjectSizeGreaterThan > 0 || rule.Filter.ObjectSizeLessThan > 0 ||
(rule.Filter.andSet && (len(rule.Filter.And.Tags) > 0 || (rule.Filter.AndSet() && (len(rule.Filter.And.Tags) > 0 ||
rule.Filter.And.ObjectSizeGreaterThan > 0 || rule.Filter.And.ObjectSizeLessThan > 0)) rule.Filter.And.ObjectSizeGreaterThan > 0 || rule.Filter.And.ObjectSizeLessThan > 0))
if hasTagOrSizeFilter { if hasTagOrSizeFilter {
continue // evaluated by lifecycle worker at scan time continue // evaluated by lifecycle worker at scan time