github.com/thiagoyeds/go-cloud@v0.26.0/internal/docs/design.md (about) 1 # Design Decisions 2 3 This document outlines important design decisions made for this repository and 4 attempts to provide succinct rationales. Recording these decisions helps 5 maintain consistency across packages, especially as an open source project where 6 contributors can join at any point during development. 7 8 A broad design goal for the Go Cloud Development Kit (Go CDK) is for the API 9 style to be consistent. Consistency aids users in building a mental model of how 10 to use the APIs. As such, the design of individual packages must also consider 11 their impact on the Go CDK as a whole. 12 13 This is a [Living Document](https://en.wikipedia.org/wiki/Living_document). The 14 decisions in here are not set in stone, but simply describe our current thinking 15 about how to guide the Go Cloud Development Kit project. While it is useful to 16 link to this document when having discussions in an issue, it is not to be used 17 as a means of closing issues without discussion at all. Discussion on an issue 18 can lead to revisions of this document. 19 20 ## Developers and Operators 21 22 The Go CDK is designed with two different personas in mind: the developer and 23 the operator. In the world of DevOps, these may be the same person. A developer 24 may be directly deploying their application into production, especially on 25 smaller teams. In a larger organization, these may be different teams entirely, 26 but working closely together. Regardless, these two personas have two very 27 different ways of looking at a Go program: 28 29 - The developer persona wants to write business logic that is agnostic of 30 underlying cloud provider. Their focus is on making software correct for the 31 requirements at hand. 32 - The operator persona wants to incorporate the business logic into the 33 organization's policies and provision resources for the logic to run. Their 34 focus is making software run predictably and reliably with the resources at 35 hand. 36 37 The Go CDK uses Go interfaces at the boundary between these two personas: a 38 developer is meant to use an interface, and an operator is meant to provide an 39 implementation of that interface. This distinction prevents the Go CDK going 40 down a path of complexity that makes application portability difficult. The 41 [`blob.Bucket`][] type is a prime example: the API does not provide a way of 42 creating a new bucket. To properly and safely create such a bucket requires 43 careful consideration, getting something like ACLs wrong could lead to a 44 catastrophic data leak. To generate the ACLs correctly requires modeling of IAM 45 users and roles for each cloud platform, and some way of mapping those users and 46 roles across clouds. While not impossible, the level of complexity and the high 47 likelihood of a leaky abstraction leads us to believe this is not the right 48 direction for the Go CDK. 49 50 Instead of adding large amounts of leaky complexity to the Go CDK, we expect the 51 operator role to handle the management of non-portable platform-specific 52 resources. An implementor of the `Bucket` interface does not need to determine 53 the content type of incoming data, as that is a developer's concern. This 54 separation of concerns allows these two personas to communicate using a shared 55 language while focusing on their respective areas of expertise. 56 57 [`blob.Bucket`]: https://godoc.org/github.com/google/go-cloud/blob#Bucket 58 59 ## Portable Types and Drivers 60 61 The portable APIs that the Go CDK exports (like [`blob.Bucket`][] or 62 [`runtimevar.Variable`][]) are concrete types, not interfaces. To understand 63 why, imagine if we used a plain interface: 64 65 ![Diagram showing user code depending on blob.Bucket, which is implemented by 66 awsblob.Bucket.](img/user-facing-type-no-driver.png) 67 68 Consider the [`Bucket.NewWriter` method][], which infers the content type of the 69 blob based on the first bytes written to it. If `blob.Bucket` was an interface, 70 each implementation of `blob.Bucket` would have to replicate this behavior 71 precisely. This does not scale: conformance tests would be needed to ensure that 72 each interface method actually behaves in the way that the docs describe. This 73 makes the interfaces hard to implement, which runs counter to the goals of the 74 project. 75 76 Instead, we follow the example of [`database/sql`][] and separate out the 77 implementation-agnostic logic from the interface. The implementation-agnostic 78 logic-containing concrete type is the **portable type**. We call the interface 79 the **driver**. Visually, it looks like this: 80 81 ![Diagram showing user code depending on blob.Bucket, which holds a 82 driver.Bucket implemented by awsblob.Bucket.](img/user-facing-type.png) 83 84 This has a number of benefits: 85 86 - The portable type can perform higher level logic without making the 87 interface complex to implement. In the blob example, the portable type's 88 `NewWriter` method can do the content type detection and then pass the final 89 result to the driver type. 90 - Methods can be added to the portable type without breaking compatibility. 91 Contrast with adding methods to an interface, which is a breaking change. 92 - When new operations on the driver are added as new optional interfaces, the 93 portable type can hide the need for type-assertions from the user. 94 95 As a rule, if a method `Foo` has the same inputs and semantics in the portable 96 type and the driver type, then the driver method may be called `Foo`, even 97 though the return signatures may differ. Otherwise, the driver method name 98 should be different to reduce confusion. 99 100 New Go CDK APIs should always follow this portable type and driver pattern. 101 102 [`runtimevar.Variable`]: 103 https://godoc.org/github.com/google/go-cloud/runtimevar#Variable 104 [`Bucket.NewWriter` method]: 105 https://godoc.org/github.com/google/go-cloud/blob#Bucket.NewWriter 106 [`database/sql`]: https://godoc.org/database/sql 107 108 ## Minimize Global State 109 110 As a library, the Go CDK should not introduce global state. Global state is 111 difficult to reason about in large codebases, where it can be necessary for 112 different parts of the application to use different states. Instead of adding 113 global state, push responsibility to the application to inject the state where 114 it is needed. 115 116 The exception we permit is URL scheme registration as documented under 117 [URLs](#urls). The amount of boilerplate setup code required for URL muxes for 118 multiple drivers without use of a tool like Wire is an unreasonable burden for 119 users of Go CDK. We want the Go CDK to be usable both with and without Wire. A 120 global registry is acceptable as long as its use is not mandatory, but the 121 burden is to prove the benefit over the cost. 122 123 ## Driver Package Naming Conventions 124 125 Inside this repository, we name packages that handle cloud services after the 126 service name, not the providing cloud (`s3blob` instead of `awsblob`). While a 127 cloud provider may provide a unique offering for a particular API, they may not 128 always provide only one, so distinguishing them in this way keeps the API 129 symbols stable over time. 130 131 The naming convention is `<provider><product><api>`, where: 132 133 * `<provider>` is the provider name, like `aws` or `gcp` or `azure`. 134 * Omit for 3rd party/open source/local packages. 135 * May also be omitted in cases where the product name is sufficient (e.g., 136 `s3blob` not `awss3blob` since S3 is well-known, `gcsblob` not 137 `gcpgcsblob` since GCS already references Google). 138 * Required if the product name is not unique across providers (e.g., 139 `gcpkms` and `awskms`). 140 * `<product>`is the product/service name. 141 * `<api>` is the portable API name. 142 * Include for local/test packages like (e.g., `fileblob`, `mempubsub`). 143 * May be omitted when it makes the package name too long (e.g. `awssnssqs` 144 is long enough, don't add `pubsub`). 145 * Encouraged when it helps distinguish the package from the service's own 146 package name (e.g., `s3blob` not `s3`). 147 148 ## Portable Type Constructors 149 150 Portable type constructors are the functions defined in driver packages that end 151 users call to get an instance of the portable type. For example, 152 `gcsblob.OpenBucket`, which returns an instance of the `*blob.Bucket` portable 153 type backed by GCS. 154 155 - Portable type constructors should be top-level functions that return the 156 portable type directly. Avoid helpers (e.g., a `Client` struct with a 157 function that returns the portable type instead of it being top-level) and 158 wrappers (e.g., a `fooblob.Bucket` type returned from `fooblob.OpenBucket` 159 that wraps the portable type). Top level functions without wrappers are 160 easier to use, especially when we're consistent about it. 161 - Order arguments that are less likely to change across multiple calls to the 162 constructor before ones that are likely to change. For example, connection 163 and authorization related arguments should go before names, so 164 `OpenBucket(ctx, client, "mybucket")` instead of `OpenBucket(ctx, 165 "mybucket", client)`. 166 - All public constructors should take an `Options` struct (see next section). 167 168 ### Option Structs 169 170 All public constructors should take an `Options` struct, even if it is currently 171 empty, to ensure that we can add arguments to the APIs in the future without 172 breaking backward compatibility. 173 174 - This includes driver constructors (e.g., `gcsblob.OpenBucket`) as well as 175 API functions (e.g., `blob.NewReader`). When in doubt, if you think it's 176 possible that we'll add arguments, add `Options`. 177 - The argument should be of type `*Options`, so that `nil` can be passed in 178 the default case. 179 - Name the `Options` struct appropriately. `Options` is usually fine for 180 portable type constructors since the package generally only exposes a 181 constructor. Inside a driver interface or in a portable type like `blob`, 182 use more descriptive names like `ReaderOptions` or `WriterOptions`. 183 - If a function already has a struct argument, don't add a separate `Options` 184 struct. Example: the various `sql.Open` functions take a `Params` struct 185 with connection parameters; we chose to add new options to `Params` instead 186 of introducing a separate `Options` struct. This keeps the function 187 signature simpler and avoid confusion about which struct new parameters 188 should be added to. 189 - When similar `Options` are part of a driver interface and also part of the 190 portable type (e.g., `blob.WriterOptions`), duplicate the struct instead of 191 aliasing or embedding it, and copy the struct fields explicitly where 192 needed. This allows the godoc for each type to be tailored to the 193 appropriate audience (e.g. end-users for the portable type, driver 194 implementors for the driver interface), and also allows the structs to 195 diverge over time if appropriate. 196 - Required arguments must not be in an `Options` struct, and all fields of the 197 `Options` struct must have reasonable defaults. Exception: struct arguments 198 that don't have `Options` in the name can contain required arguments (e.g., 199 see the `Params` example for `sql.Open` above). 200 201 Regarding empty `Options` structs: we considered only adding them when the first 202 option is added, and using a separate constructor for compatibility (e.g., start 203 with `foo.New(...)` and later add `foo.NewWithOptions(..., opts *Options)` if 204 needed). However, this would result in inconsistent names over time (e.g., some 205 packages would expose `New` with an `Options`, while others would expose 206 `NewWithOptions`). 207 208 ### Compound IDs 209 210 Many cloud providers have resource IDs that are made up of subcomponents in 211 some well-defined syntax. For example, [GCP KMS key IDs][] take the form 212 `projects/[PROJECT_ID]/locations/[LOCATION]/keyRings/[KEY_RING]/cryptoKeys/[KEY]`. 213 We call these _compound IDs_. 214 215 There are two broad compound ID usage patterns we have observed: 216 217 1. Applications will keep resources in the same location, so the application 218 will build the ID from the subcomponents rather than passing the entire 219 resource ID around. 220 2. Applications will pass a verbatim string from configuration down to the 221 API, since this is what was easily copy-pasteable from the cloud console UI. 222 223 Go CDK constructors that take in compound IDs should take in a `string` with the 224 full compound ID. Helper functions to build these compound IDs from 225 subcomponents may be provided as needed. URL openers (described below) should 226 prefer to use the full compound ID in their URL format. 227 228 [GCP KMS key IDs]: https://cloud.google.com/kms/docs/object-hierarchy#key 229 230 ### URLs 231 232 To enable the [Backing services factor][] of a Twelve-Factor Application, Go 233 Cloud includes the ability to construct each of its API objects using 234 identifying URLs. The portable type's package should include APIs like the 235 following: 236 237 ```go 238 // Package foo is a portable API. foo could be something like blob or pubsub. 239 // 240 // Throughout this example, Widget is used as a stand-in for a portable type 241 // inside foo, like Bucket or Subscription. 242 package foo 243 244 // A type that implements WidgetURLOpener can open widgets based on a URL. 245 // The opener must not modify the URL argument. OpenWidgetURL must be safe to 246 // call from multiple goroutines. 247 // 248 // WidgetURLOpeners should not assume that the URL has a particular scheme. 249 type WidgetURLOpener interface { 250 OpenWidgetURL(ctx context.Context, u *url.URL) (*Widget, error) 251 } 252 253 // URLMux is a URL opener multiplexer. It matches the scheme of the URLs 254 // against a set of registered schemes and calls the opener that matches the 255 // URL's scheme. 256 // 257 // The zero value is a multiplexer with no registered schemes. 258 type URLMux struct { 259 // ... 260 } 261 262 // RegisterWidget registers the opener with the given scheme. If an opener 263 // already exists for the scheme, RegisterWidget panics. 264 func (mux *URLMux) RegisterWidget(scheme string, opener WidgetURLOpener) { 265 // ... 266 } 267 268 // OpenWidget calls OpenWidgetURL with the URL parsed from urlstr. 269 // OpenWidget is safe to call from multiple goroutines. 270 func (mux *URLMux) OpenWidget(ctx context.Context, urlstr string) (*Widget, error) { 271 u, err := url.Parse(urlstr) 272 if err != nil { 273 return nil, fmt.Errorf("open widget: %v", err) 274 } 275 return mux.OpenWidgetURL(ctx, u) 276 } 277 278 // OpenWidgetURL dispatches the URL to the opener that is registered with the 279 // URL's scheme. OpenWidgetURL is safe to call from multiple goroutines. 280 func (mux *URLMux) OpenWidgetURL(ctx context.Context, u *url.URL) (*Widget, error) { 281 // ... 282 } 283 284 // DefaultURLMux returns the URLMux used by OpenWidget. 285 func DefaultURLMux() *URLMux { 286 return defaultURLMux 287 } 288 289 var defaultURLMux = new(URLMux) 290 291 // OpenWidget opens the Widget identified by the URL given. URL openers must be 292 // registered in the DefaultURLMux, which is typically done in driver 293 // packages' initialization. 294 func OpenWidget(ctx context.Context, urlstr string) (*Widget, error) { 295 return DefaultURLMux().OpenWidget(urlstr) 296 } 297 ``` 298 299 The repetition of `Widget` in the method names permits a type to handle multiple 300 resources within the API. Exporting the `URLMux` allows applications to build 301 their own muxes, potentially wrapping existing ones. 302 303 Driver packages should include their own `URLOpener` struct type which 304 implements all the relevant `WidgetURLOpener` methods. The URL should only serve 305 to identify which resource to open. Any credentials or other complex values 306 should be taken in as struct fields, not as input from URL. If the driver 307 package registers its `URLOpener` with the `DefaultURLMux`, then it should 308 populate these complex fields from environment variables. If doing so is 309 undesirable or expensive, then it should not register with the `DefaultURLMux` 310 and instead rely on users to create their own mux. If there already exists a 311 well-established URI format for the backend (like S3 URLs or database connection 312 URIs), then drivers should honor them where possible. 313 314 [Backing services factor]: https://12factor.net/backing-services 315 316 #### URL Examples 317 318 A `WidgetURLOpener` implementation for a hypothetical GCP service: 319 320 ```go 321 package gcpfoo 322 323 // ... 324 325 const Scheme = "gcpwidget" 326 327 type URLOpener struct { 328 Client *gcp.HTTPClient 329 Options Options 330 } 331 332 func (o *URLOpener) OpenWidgetURL(ctx context.Context, u *url.URL) (*foo.Widget, error) { 333 // ... 334 return OpenWidget(ctx, o.Client, u.Host, &o.Options) 335 } 336 337 type lazyURLOpener struct { 338 init sync.Once 339 opener *URLOpener 340 err error 341 } 342 343 func (o *lazyURLOpener) OpenWidgetURL(ctx context.Context, u *url.URL) (*foo.Widget, error) { 344 o.init.Once(func() { 345 creds, err := gcp.DefaultCredentials(ctx) 346 if err != nil { 347 o.err = err 348 return 349 } 350 o.opener = new(URLOpener) 351 o.opener.Client, _ = gcp.NewHTTPClient(http.DefaultTransport, creds.TokenSource) 352 }) 353 if o.err != nil { 354 return nil, o.err 355 } 356 return o.opener.OpenWidgetURL(ctx, u) 357 } 358 359 func init() { 360 foo.DefaultURLMux().Register(Scheme, new(lazyURLOpener)) 361 } 362 363 // OpenWidget is the exported non-URL constructor. 364 func OpenWidget(ctx context.Context, c *gcp.HTTPClient, name string, opts *Options) (*foo.Widget, error) { 365 // ... 366 } 367 ``` 368 369 Using the global default mux: 370 371 ```go 372 import _ "gocloud.dev/foo/gcpfoo" 373 374 // ... 375 376 widget, err := foo.OpenWidget(context.Background(), "gcpwidget://xyzzy") 377 ``` 378 379 Using a custom mux created during server initialization: 380 381 ```go 382 myMux := new(foo.URLMux) 383 myMux.Register(gcpfoo.Scheme, &gcpfoo.URLOpener{ 384 Client: client, 385 }) 386 widget, err := myMux.OpenWidget(context.Background(), "gcpwidget://xyzzy") 387 ``` 388 389 ## Errors 390 391 ### General 392 393 - The callee is expected to return `error`s with messages that include 394 information about the particular call, as opposed to the caller adding this 395 information. This aligns with common Go practice. 396 397 ### Drivers 398 399 Driver implementations should: 400 401 - Return the raw errors from the underlying service, and not wrap them in 402 `fmt.Errorf` calls, so that they can be exposed to end users via `ErrorAs`. 403 404 ### Portable Types 405 406 Portable types should: 407 408 - Wrap errors returned from driver implementations before returning them to 409 end users, so that users can't peek into driver-specific error details 410 without using `As`. Make sure not to double-wrap. 411 412 - Use `internal/gcerr.New` when wrapping driver errors, like so: `if err := 413 driver.Call(xyz); err != nil { return gcerr.New(code, err, 1, "blob") }` The 414 first argument is an error code. See below for advice on choosing the 415 appropriate code. 416 417 The third argument is the distance in stack frames from the function whose 418 location should be associated with the error. It should be `1` if you are 419 calling `New` from the same function that made the driver call, `2` if you 420 are calling new from a helper function, and so on. The fourth argument is an 421 additional string that will display with the error. You should pass the API 422 name. 423 424 - By default, choose the code `Unknown`, keeping details of returned `error`s 425 unspecified. The most common case is that the caller will only care whether 426 an operation succeeds or not. 427 428 - If certain `error`s are interesting for callers to distinguish, choose one 429 of the other codes from the `gcerrors.ErrorCode` enum, so user programs can 430 act on the kind of error without having to look at driver-specific errors. 431 432 - If more than one error code makes sense, choose the most specific one. 433 - If none make sense, choose `Unknown`. 434 - If none make sense but you want something more specific than `Unknown`: 435 - If you can generalize your code to make it applicable to more than 436 just your API, add it to `gcerrors.ErrorCode`. Look at the 437 [gRPC error codes](https://github.com/grpc/grpc-go/blob/master/codes/codes.go) 438 for inspiration. 439 - Otherwise, you can define a custom code in your portable API 440 package. Your code should use a negative integer. 441 442 - For now, your package should expose an `ErrorAs` function to allow users to 443 access driver-specific error types. We may review this choice if 444 `golang.org/x/xerrors.As` becomes part of the standard library. 445 446 - Handle transient network errors. Retry logic is best handled as low in the 447 stack as possible to avoid [cascading failure][]. APIs should try to surface 448 "permanent" errors (e.g. malformed request, bad permissions) where 449 appropriate so that application logic does not attempt to retry 450 non-idempotent operations, but the responsibility is largely on the library, 451 not on the application. 452 453 [cascading failure]: 454 https://landing.google.com/sre/book/chapters/addressing-cascading-failures.html 455 456 ## Escape Hatches using As 457 458 The Go CDK allows users to escape the abstraction as needed using `As` 459 functions, described in more detail in the 460 [concept guide](https://gocloud.dev/concepts/as/). `As` functions take an 461 `interface{}` and return a `bool`; they return `true` if the underlying concrete 462 type could be converted into the type provided as the `interface{}`. 463 464 An alternative approach would have been something like 465 [`os.ProcessState.Sys`](https://golang.org/pkg/os/#ProcessState.Sys), which 466 returns an `interface{}` that the user can then type cast/assert to 467 service-specific types. 468 469 We ended up going with `As` because: 470 471 1. Most portable types have an `As` function for errors; choosing `As` results 472 in an easy and natural implementation for chained errors once the 473 [Go 2 proposal for errors](https://go.googlesource.com/proposal/+/master/design/29934-error-values.md) 474 arrives. It is currently implemented in 475 [xerrors](https://godoc.org/golang.org/x/xerrors#As), and we're already 476 using that in some drivers. 477 2. `As` adds more flexibility for drivers to support conversions to multiple 478 types. Specifically, not the case where there are multiple possible 479 underlying types, but rather that a single underlying type can be converted 480 to multiple types. 481 * Chained errors is one example of this, where the top-level error may 482 always be the same type, but may also represent a chain of other errors 483 with different types. 484 * Another example is that a driver might choose to support `As`-level 485 compatibility with another driver; e.g., driver `foo` could support all 486 of the `As` types defined by `s3blob`, converting them internally, and 487 then any code that runs with driver `s3blob` would also work with driver 488 `foo` (even if it uses the `As` escape hatches). 489 490 ## Enforcing Portability 491 492 The Go CDK APIs will end up exposing functionality that is not supported by all 493 services. In addition, some functionality details will differ across services. 494 Some theoretical examples using [`blob.Bucket`][]: 495 496 1. **Top-level APIs**: There might be a service that supports reads, but not 497 writes or deletes. 498 1. **Data fields**. Some services may support key/value metadata associated 499 with a blob, others may not. 500 1. **Naming rules**. Different services may allow different name lengths, or 501 allow/disallow non-ASCII unicode characters. See [Strings](#strings) below 502 for more on handling string differences. 503 1. **Semantic guarantees**. Different services may have different consistency 504 guarantees; for example, S3 only provides eventually consistency while GCS 505 provides strong consistency. 506 507 How can we maintain portability while these differences exist? 508 509 ### Guiding Principle 510 511 Any incompatibilities between drivers should be visible to the user as soon as 512 possible. From best to worst: 513 514 1. At compile time 515 1. At configuration/app startup time (e.g., when the portable type is created) 516 1. At runtime (e.g., when the incompatible behavior is accessed), via a non-nil 517 error 518 1. At runtime, via panic 519 520 ### Approaches Considered 521 522 1. **Documentation**. We could try to document non-uniform or optional 523 functionality across drivers. Optional fields or functionality would 524 return "not implemented" errors or zero values. 525 1. **Restrict functionality to the intersection**. We could explicitly only 526 support the intersection of all services. For example, if not all services 527 allow unicode characters in names, then **blob** would not allow it either. 528 1. **Enforced feature codes**: Go CDK APIs could enumerate the ways in which 529 drivers differ as a `FeatureCode` enum. 530 * Drivers would declare which feature codes they support, enforced by 531 extensions to the existing conformance tests. 532 * API users would declare which feature codes they need. 533 * Mismatches between what a user requests and what the driver supports 534 would be enforced at initialization time. 535 * As much as possible, the API (via the portable type) would enforce that 536 the user is only exposed to optional functionality that they asked for. 537 * For example, the default legal name for a blob might be ASCII only, with 538 a `FeatureUnicodeNames` feature code. Users that don't request this 539 feature code would only be able to use blobs with ASCII names, even if 540 the underlying service supports unicode. If the user requested 541 `FeatureUnicodeNames`, and their driver supports it, they could then 542 use blobs with unicode; if their driver doesn't support it, they would 543 get an initialization-time error. 544 545 ``` 546 b, err := blob.NewBucket(d, blob.FeatureUnicodeNames) 547 ... 548 ``` 549 550 Design discussions regarding enforcing portability are ongoing; we welcome input 551 on the [mailing list](https://groups.google.com/forum/#!forum/go-cloud). 552 553 ### Strings 554 555 Services often differ on what they accept in particular strings (e.g., blob 556 names, metadata keys, etc.). A couple of specific examples: 557 558 * Azure Blob only 559 [accepts C# identifiers](https://docs.microsoft.com/en-us/azure/storage/blobs/storage-properties-metadata) 560 as metadata keys. 561 * S3 drops double slashes in blob names (e.g., `foo//bar` will end up being 562 saved as `foo/bar`). 563 564 These differences lead to a loss of portability and predictability for users. 565 566 To resolve this issue, we insist that Go CDK can handle any UTF-8 string, and 567 force drivers to use escaping mechanisms to handle strings that the underlying 568 service can't handle. We enforce driver compliance with conformance tests. 569 Behavior for non-UTF-8 strings is undefined (but see 570 https://github.com/google/go-cloud/issues/1281 and 571 https://github.com/google/go-cloud/issues/1260). 572 573 We try to use URL encoding as the escaping mechanism where possible; however, 574 sometimes it is not and we'll use custom escaping. As an example, a driver for a 575 service that only allows underscores and ASCII alphanumeric characters might 576 escape the string `foo.bar` to `foo__0x2e__bar` (URL escaping won't work because 577 `%` isn't allowed). 578 579 Pros of this approach: 580 581 * Go CDK APIs are internally consistent in that a user can write any string to 582 any service and get the original string back when they read it back. 583 * Go CDK APIs have visibility into all existing strings for all services. 584 585 Cons: 586 587 * Go CDK could overwrite existing data if a Go CDK-written key escapes to an 588 already-existing value (e.g., if the `foo__0x2e__bar` string already 589 existed, it would be overwritten by a Go CDK write to `foo.bar`). 590 * Escaping may push a string over the maximum allowed string length for a 591 service. Escaping does not solve (and in fact may exacerbate) problems with 592 different maximum string lengths across services. 593 * Existing strings that happen to look like Go CDK-escaped strings will be 594 unescaped by Go CDK (e.g., an existing string `foo__0x2e__bar` would appear 595 as `foo.bar` when read through the Go CDK). 596 * Strings that were written through the Go CDK and needed escaping will appear 597 in their escaped form when viewed outside of Go CDK (e.g., `foo__0x2e__bar` 598 would appear on the service's UI). 599 600 Most of these cons are mitigated by choosing unusual-looking escape mechanisms 601 that are unlikely to appear in existing data. 602 603 Drivers should escape strings when writing to the underlying service, and 604 unescape them when reading them back. The Go CDK will provide helpers for these 605 operations, as well as a test suite of strings for conformance tests. 606 607 Sample code for the helper for escaping strings: 608 609 ``` 610 // package escape provides helpers for escaping and unescaping strings. 611 package escape 612 613 // Escape returns s, with all runes for which shouldEscape returns true 614 // escaped to "__0xXXXX__", where XXXX is the hex representation of the rune 615 // value. For example, " " would escape to "__0x20__". 616 // 617 // Non-UTF-8 strings will have their non-UTF-8 characters escaped to 618 // unicode.ReplacementChar; the original value is lost. Please file an 619 // issue if you need non-UTF8 support. 620 // 621 // Note: shouldEscape takes the whole string as a slice of runes and an 622 // index. Passing it a single byte or a single rune doesn't provide 623 // enough context for some escape decisions; for example, the caller might 624 // want to escape the second "/" in "//" but not the first one. 625 // We pass a slice of runes instead of the string or a slice of bytes 626 // because some decisions will be made on a rune basis (e.g., encode 627 // all non-ASCII runes). 628 func Escape(s string, shouldEscape func(s []rune, i int) bool) string { ... } 629 630 // Unescape reverses Escape. 631 func Unescape(s string) string {...} 632 ``` 633 634 Sample code for how a driver might use it, using metadata keys for a `blob` as 635 the example string: 636 637 ``` 638 // When writing metadata keys, escape the keys: 639 // ... gcdkMetadata is the metadata passed to the GCDK API. 640 for k, v := range gcdkMetadata { 641 e := escape.Escape(k, func (r []rune, i int) bool {...}) 642 if _, ok := serviceMetadata[e]; ok { 643 return fmt.Errorf("duplicate keys after escaping: %q => %q", k, e) 644 } 645 serviceMetadata[e] = v 646 } 647 // ... write serviceMetadata to the service. 648 649 // When reading metadata keys, unescape them: 650 // ... serviceMetadata is the metadata read from the service. 651 for k, v := range serviceMetadata { 652 gcdkMetadata[escape.Unescape(k)] = v 653 } 654 // ... return gcdkMetadata. 655 ``` 656 657 The details of what runes need to be escaped will vary from service to 658 service. The details of how to escape may also vary, although we expect to use 659 URL encoding where possible, and a common custom escaping where not. For the 660 custom escaping, we plan to escape each rune for which `shouldEscape` returns 661 true with `__0xXXX__`, where `XCX` is the hex representation of the rune value. 662 663 ### Alternatives Considered 664 665 * We considered restricting Go CDK's APIs to strings that all services 666 support. For example, we could have asserted that Go CDK's `blob` only 667 supports ASCII plus `/` for blob names (and no `//`!). However, such a rule 668 would mean that we couldn't cleanly handle existing strings created through 669 some mechanism other than through Go CDK APIs that violate the rule. For 670 example, an existing blob in S3 with a unicode name. Filtering out such 671 strings so that they aren't visible at all through the Go CDK would be both 672 surprising and limiting, and could easily result in data loss (e.g., if a 673 user read a set of metadata for a blob via the Go CDK, and some keys were 674 filtered out, and then wrote the metadata back, the filtered keys would be 675 lost). Not filtering such strings would mean that the Go CDK isn't 676 internally consistent (i.e., you can read some strings but not write them). 677 Overall, we decided that this approach is unacceptable. 678 679 * We could expose the escaper used by drivers in their `Options` structs 680 (including options like disabling it, overriding the set of bytes to be 681 escaped, or overriding the escaping mechanism), but we'll wait to see if 682 there's demand for that. 683 684 ## Coding Conventions 685 686 We try to adhere to commonly accepted Go coding conventions, some of which are 687 described on the 688 [Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) 689 wiki page. We also adopt the following guidelines: 690 691 - Prefer `map[K]V{}` to `make(map[K]V)`. It's more concise. 692 - When writing a loop appending to a slice `s`, prefer 693 694 ``` 695 var s []T 696 for ... { 697 ... 698 s = append(s, ...) 699 ... 700 } 701 ``` 702 703 to 704 705 ``` 706 s := make([]T, 0, N) 707 for ... { 708 ... 709 s = append(s, ...) 710 ... 711 } 712 ``` 713 714 or 715 716 ``` 717 s := make([]T, N) 718 for ... { 719 ... 720 s[i] = ... 721 ... 722 } 723 ``` 724 725 (Exception: the loop body is trivial and the loop is performance-sensitive.) 726 The first version is shorter and easier to read, and it is impossible to get 727 the length wrong. 728 729 - Prefer `log.Fatal` to `panic` in example tests. 730 731 - Ensure you've run `goimports` on your code to properly group import 732 statements. 733 734 - Order arguments that are less likely to change across multiple calls to the 735 constructor before ones that are likely to change. For example, connection 736 and authorization related arguments should go before names, so 737 `OpenBucket(ctx, client, "mybucket")` instead of `OpenBucket(ctx, 738 "mybucket", client)`. 739 740 ## Tests 741 742 ### Conformance Tests 743 744 Since our goal is for users to be able to use drivers interchangeably, it is 745 critical that they behave similarly. To this end, each portable API (e.g., 746 `blob`) must provide a suite of conformance tests that driver implementations 747 should run. The conformance tests should be comprehensive; drivers should not 748 need additional unit tests for the core driver semantics. 749 750 ### Provisioning For Tests 751 752 Portable API integration tests require developer-specific resources to be 753 created and destroyed. We use [Terraform](http://terraform.io) to do so, and 754 record the resource info and network interactions so that they can be replayed 755 as fast and repeatable unit tests. 756 757 ### Replay Mode 758 759 Tests normally run in replay mode. In this mode, they don't require any 760 provisioned resources or network interactions. Replay tests verify that: 761 762 - The same test inputs produce the same requests (e.g., HTTP requests) to the 763 cloud service. Some parts of the request may be dynamic (e.g., dates in the 764 HTTP request headers), so the replay tests do some scrubbing when verifying 765 that requests match. Some parts of this scrubbing are service-specific. 766 767 - The replayed service responses produce the expected results from the 768 portable API library. 769 770 ### Record Mode 771 772 In `-record` mode, tests run as integration tests, making live requests to 773 backend servers and recording the requests/responses for later use in replay 774 mode. 775 776 To use `-record`: 777 778 1. Provision resources. 779 780 - For example, the tests for the AWS implementation of Blob requires a 781 bucket to be provisioned. 782 - TODO(issue #300): Use Terraform scripts to provision the resources 783 needed for a given test. 784 - For now, do this manually. 785 786 2. Run the test with `-record`. 787 788 - TODO(issue #300): The test will read the Terraform output to find its 789 inputs. 790 - For now, pass the required resources via test-specific flags. 791 - When changing or adding tests, please only record the tests that are 792 changed/affected by passing the `-run` flag to `go test` with the 793 name of the test(s). Re-recording all tests of a driver creates a lot 794 of noise and a large diff that's difficult to review. 795 796 3. The test will save the network interactions for subsequent replays. 797 798 - TODO(issue #300): The test will save the Terraform output to a file in 799 order to replay using the same inputs. 800 - Commit the new replay files along with your code change. Expect to see 801 lots of diffs; see below for more discussion. 802 803 ### Diffs in replay files 804 805 Each time portable API tests are run in `-record` mode, the resulting replay 806 files are different. Looking at diffs of these files isn't particularly useful. 807 808 We [considered](https://github.com/google/go-cloud/issues/276) trying to scrub 809 the files of dynamic data so that diffs would be useful. We ended up deciding 810 not to do this, for several reasons: 811 812 - There's a lot of dynamic data, in structured data of various forms (e.g., 813 HTTP headers, XML/JSON body, etc.). It would be difficult and fragile to 814 scrub it all. 815 816 - The scrub process would also be fragile relative to changes in services 817 (e.g., adding a new dynamic HTTP response header). 818 819 - The scrub process would need to be implemented for every new service, 820 increasing the barrier to entry for new implementations. 821 822 - Scrubbing would likely be even more difficult for services using a 823 non-HTTP-based protocol (e.g., gRPC). 824 825 - Scrubbing the data decreases the fidelity of the replay test, since it 826 wouldn't be operating on the original data. 827 828 Overall, massive diffs in the replay files are expected and fine. As part of a 829 code change, you may want to check for things like the number of RPCs made to 830 identify performance regressions.