Add OCI Object Storage: buckets, objects, multipart, PARs - #422
Add OCI Object Storage: buckets, objects, multipart, PARs#422arunesh-j wants to merge 2 commits into
Conversation
NitinKumar004
left a comment
NitinKumar004
left a comment
There was a problem hiding this comment.
Review notes
Real data-plane engine (per #427): N/A — OCI Object Storage is object storage, which is explicitly NOT engine-eligible under the #427 model (ComputeEngine/DatabaseEngine/CacheEngine/FunctionEngine/ContainerEngine).
Findings
High · wire-fidelity — GetNamespaceMetadata (GET /n/{ns}) is misrouted to ListBuckets when the tenancy namespace begins with 'b'
server/oci/objectstorage/handler.go:287
If a user's tenancy hashes to a namespace starting with 'b' -> every GetNamespaceMetadata call (GET /n/{ns}) returns 400 'compartmentId is required' instead of the metadata body -> S3/Swift-compat setup and any SDK flow that reads namespace metadata breaks non-deterministically for ~6% of tenancies, while the identical code works for everyone else. Fix: carry a 'hadBucketSegment' bool out of parseNamespaced instead of re-sniffing the raw path.
serveBucketCollection distinguishes /n/{ns} from /n/{ns}/b with strings.Contains(r.URL.Path, "/"+segBuckets) (segBuckets=="b"). The parsed route already discards whether a /b segment was present, so this substring heuristic is the only signal. For a namespace whose first char is 'b', the path "/n/b25a97828eed" contains the substring "/b", so the request falls through to the GET branch = listBuckets, which requires compartmentId. Reproduced with tenancy "ocid1.tenancy.oc1..probe0100" -> namespace
Medium · docs — docs/services.md not updated with OCI Object Storage (Definition-of-done item)
docs/services.md:59
If a reader consults docs/services.md to learn which OCI services cloudemu emulates -> Object Storage is invisible there even though it is fully implemented -> the service is undiscoverable via the canonical human-facing doc, and the PR fails a stated done-criterion (a future migration-drift CI check keys off this file).
docs/oci-conventions.md Definition of done requires 'Operations added to docs/services.md'. The storage section header (line 59) still reads 'AWS: S3 | Azure: Blob Storage | GCP: GCS' with no 'OCI: ObjectStorage' entry, unlike networking (line 407 'OCI: VCN ...'), OCI Monitoring (623) and Identity (670) which each have OCI subsections. Only the generated docs/coverage/* files were updated in the diff; the hand-maintained services.md was not touched.
Medium · coverage — Changed packages below the 90% pillar; lifecycle-expiry and versioning wrappers untested
providers/oci/objectstorage/retention.go:366
If EvaluateLifecycle mis-evaluates expiry (e.g. off-by-one on the ExpirationDays*24h window, or a prefix mismatch) -> a caller relying on lifecycle-expiry reporting gets wrong object lists -> the regression ships silently because no test exercises the aged-out branch. Add a FakeClock test that advances past ExpirationDays and asserts the expired name, plus a HeadObjectVersion and portable-versioning round-trip.
go test -cover: providers/oci/objectstorage 66.2%, server/oci/objectstorage 71.6% (pillar target 90%). 0%-covered flows include EvaluateLifecycle + objectExpired (retention.go:366/399 — object age-out evaluation is never asserted by any test), HeadObjectVersion (versioning.go:228), and the portable SetBucketVersioning/GetBucketVersioning wrappers (versioning.go:140/159). TestBucketLifecycleCRUD stores and reads a policy but never drives an object past its ExpirationDays to confirm EvaluateLifecy
Low · wire-fidelity — ListObjects default page size is 100, not OCI's 1000
server/oci/objectstorage/object.go:233
If a client lists a bucket of 500 objects without an explicit limit -> it receives 100 + a nextStartWith cursor and must paginate 5x -> extra round-trips and a subtle behavioral difference from real OCI. Minor and partly a shared-ocirest.DefaultLimit choice; note only.
listOptions sets MaxKeys: ocirest.Limit(r), which returns ocirest.DefaultLimit (100) when 'limit' is absent and is always >=1, so the provider's defaultListLimit (1000, matching real OCI) at object.go:15 is never reached from the wire path. Real OCI ListObjects returns up to 1000 per page by default.
Low · coverage — No oci-go-sdk SDK-compat test
server/oci/objectstorage/handler_test.go:1
If the handler's wire shape drifts from what the real SDK emits/expects (header casing, envelope fields) -> hand-rolled tests that mirror the handler's own assumptions won't catch it -> a real SDK user hits the mismatch first. Recommended, not blocking.
Convention: 'An SDK-compat test using github.com/oracle/oci-go-sdk against httptest.NewServer is the strongest evidence the handler is right. Add one where the SDK makes it practical.' All wire tests are hand-rolled httptest requests; no oracle/oci-go-sdk client is driven against the handler.
Implements OCI Object Storage against the portable storage driver.
providers/oci/objectstorage holds the Mock over memstore.Store, satisfying
driver.Bucket and the optional driver.VersionedBucket. Buckets carry OCI's
settings (public access type, storage tier, versioning tri-state, KMS key,
auto-tiering) and record the compartment they were created in. Objects carry
opc-meta- user metadata, a content MD5 and a per-object storage tier.
Retention rules hold objects against delete and overwrite, and a locked rule
can only be extended. A pre-authenticated request is a first-class resource
with its own OCID, lifetime and redemption token, so GeneratePresignedURL
creates a real PAR that ListPARs and DeletePAR can see and revoke.
server/oci/objectstorage serves the /n/{namespace}/b/{bucket}/o/{object}
surface, plus multipart uploads, object versions, retention rules, the
lifecycle policy, PAR management and PAR redemption at /p/{token}/n/…. The
OCI-only behaviour is a consumer-side Extras interface satisfied by the mock;
a driver that does not satisfy it is served 501. ListBuckets requires
compartmentId; the bucket-scoped lists take what real OCI takes. copyObject
is asynchronous and records a work request, as real OCI does.
Operations with no OCI equivalent are named rather than silently accepted:
bucket policies (Identity policies do that), CORS, object tags (objects carry
opc-meta- metadata), reencrypt and restoreObjects, and multipart
partsToExclude.
… add persistence
Review follow-ups on the OCI Object Storage PR.
GetNamespaceMetadata misrouted to ListBuckets whenever the tenancy namespace
began with "b": serveBucketCollection separated /n/{ns} from /n/{ns}/b by
sniffing the raw path for the substring "/b", so /n/b25a97828eed matched and
fell through to listBuckets, which 400s without compartmentId. parseNamespaced
now carries whether a /b segment was actually parsed.
HeadObject reported neither Content-Length nor the object's Content-Type: a HEAD
carries no body, so a client had no way to learn the size. It now answers
through its own writer rather than the JSON helper, whose application/json was
overwriting the object's type.
ListObjects applied ocirest.DefaultLimit (100) when the caller named no limit,
so the provider's own 1000 — real OCI's page size — was unreachable from the
wire. An absent limit is now left for the provider to fill in, leaving
DefaultLimit alone for the other OCI services.
Object bytes now flow through config.WithStorageEngine, the seam AWS S3, Azure
Blob and GCP GCS already use, keyed by object version so each version is
addressed separately. Object and version records track Size independently, so
Head, List and a bucket's approximate size stay correct once the bytes are
offloaded.
Adds Snapshottable, which persist's completeness guard (#582) now requires:
buckets, objects, version chains, PARs (with their redemption tokens, so an
access URI issued before a snapshot still redeems), retention rules and the
lifecycle policy round-trip under their original identities.
Object Storage metrics were silently dropped: the provider publishes to
oci_objectstorage, and the OCI Monitoring mock refused any Oracle-reserved
namespace on every path. The reservation now applies to PostMetricData, the
customer-facing one, and not to the seam Oracle's own emulated services use.
Adds --oci-tenancy so the tenancy — and therefore the Object Storage namespace
derived from it — is reachable from the CLI, as the AWS account, Azure
subscription and GCP project already are.
Documents the service in docs/services.md, and raises coverage to 93.9%
(provider) and 94.9% (wire) from 66.2% and 71.6%.
b85c29f to
f285446
Compare
|
Rebased onto current High —
|
2 participants
Summary
storagedriver.services/storage/driver— OCI-only behaviour is a consumer-sideExtrasinterface, per the rule set in Move OCI-only capabilities out of shared driver packages #393.Closes #410. Part of #376.
Changes
providers/oci/objectstorage/—Mockovermemstoreimplementingdriver.Bucket, guarded by async.RWMutex: namespace derivation, buckets, objects, versioning, multipart, retention, PARs.server/oci/objectstorage/— the/n/{namespace}/b/{bucket}/o/{object}surface plus PAR redemption at/p/{token}/n/….providers/oci/oci.goandserver/oci/oci.go.Operations
Namespace + metadata; bucket create/get/head/update/delete/list; object put/get/head/delete/list (prefix, delimiter, paging); rename and
updateObjectStorageTier; asynccopyObject; multipart create/upload/list-parts/commit/abort/list; object versions (Disabled/Enabled/Suspended, version-addressable GET/HEAD/DELETE, delete markers,objectversions); retention rules with lock semantics; lifecycle policy PUT/GET/DELETE; PARs including redemption.PARs are modelled as real resources
A PAR gets its own OCID,
timeExpires, an opaque redemption token, and is listable and revocable — not a fabricated signature.GeneratePresignedURLcreates one, so the URL it returns actually works and can be revoked. Demonstrated in the transcript: read-only PAR servesGET, refusesPUTwith 403, and 404s after revocation.Compartment scoping — one deliberate narrowing
Only
ListBucketscallsRequireCompartmentID; it is the one collection real OCI scopes by compartment. Object, PAR, retention and upload lists are bucket-scoped and take nocompartmentId, so requiring it would reject calls real OCI accepts.CreateBucketrequires it in the body. Stated in the package doc.Never accept-and-ignore
Rejected by name rather than silently dropped: bucket policies (→ Identity policies), CORS, object tags (→
opc-meta-),reencrypt,restoreObjects, multipartpartsToExclude, cross-namespace copy, more than one lifecycleinclusionPrefix, disabling encryption, versioning back toDisabled, and unknown access types / tiers / time units.reencryptis a 501 rather than a 202: with no per-object key material there is nothing to re-wrap, and a 202 there would be theatre.Provider Coverage
Checklist
go test ./...) — exit 0, 272 packagesgolangci-lint run --timeout=9m) — 0 issuescloudemu_test.go— driver + handler tests insteadTest Plan
Coverage leak check clean: no OCI operation appears in
docs/coverage/{aws,azure,gcp}/*.md, andgit diff development -- services/is empty.End-to-end on a running server (port 4611):
Note on the suite
Other Wave 2 branches see a pre-existing failure in
cmd/cloudemu TestServeOutOfProcess("AWS endpoint never became ready") which reproduces on cleandevelopment— it spawns a child process that cannot bind a listener in a sandbox. It passed on this run; flagging it as flaky-environmental rather than related to this change.