-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
rfc: Cloud Assembly Specification #1119
Conversation
Looking forward to supporting a more integrated deployment experience with support of multiple stacks (with deployment order constraints honored) as well as assets, this proposes a specification for a *Cloud Assembly* that determines a document storage format. Fixes: #956
3bec2b0
to
a43db28
Compare
specifications/cloud_assembly.md
Outdated
including all the parts that are needed in order to deploy those to *the cloud*. This document exposes the specification | ||
of the *Cloud Assembly* format as well as requirements imposed on *Cloud Assemblers* and *Cloud Runtimes*. | ||
|
||
### Goals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be "tenets" (or "design goals"). To write good tenets, a nice trick I've learned is to think (and even describe) what it is NOT. The purpose of tenets is to help make design decisions. Each tenet should describe a dimension or continuum and state which side of the continuum we are going to prefer.
Also, there's a bit of conflation here between goals for the spec it self (e.g. "cloud agnostic", there's no goal for assemblies to be cloud agnostic) and the assembly (e.g. "extensible", there isn't a goal for the spec to be extensible).
Anyway - this needs a bit more work, happy to spend some face to face time on this if you like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I very much would like the specification to be fundamentally cloud-agnostic, which is why I use "the cloud" and not "AWS". As for extensibility, this may not be the right term indeed (settled for this now until I figure out a better one).
|
||
Documents in the archive can be stored with any name and directory structure, however the following entries at the root | ||
of the archive are reserved for special use: | ||
* `manifest.json` **MUST** be present and contains the [manifest document](#manifest-document) for the *Cloud Assembly*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe index.json
? Give it a bit of a "web standard" ora
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If it's a cat, call it a cat".
The [JSON] specification allows for keys to be specified multiple times in a given `object`. However, *Cloud Assembly* | ||
consumers **MAY** assume keys are unique, and [*Cloud Assemblers*](#cloud-assemblers) **SHOULD** avoid generating | ||
duplicate keys. If duplicate keys are present, the latest specified value **SHOULD** be preferred. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include a JSON schema and/or a typescript type definition (preferably the former generated from the latter)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, that was always the plan :P
specifications/cloud_assembly.md
Outdated
|
||
Each [`Drop` Type](#drop-type) can produce outputs that can be used in order to allow other `Drop`s to consume the | ||
resources they procude. Each `Drop` implementer is responsible to document the output attributes it supports. References | ||
to these outputs are modeled using special `string` tokens within entries of the `properties` section of `Drop`s: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec is missing a section on how to publish new drop types. Specifically, it should include a JSON schema which allows tools to validate the manifest "properties" section against
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a part of it but I reckon there might be more to it than just this. It'll be pretty inefficient to discuss over GitHub issues, though. Let's talk about this & report the outcome back here.
Deployment systems **MUST** support at least one protocol, and **SHOULD** support all the protocols specified in | ||
the following sub-sections. | ||
|
||
##### The `npm` protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks nice, but I am wondering if it's at all possible to implement a deployment tool that can interact with drop types from multiple languages.
Perhaps the drop type provider is not a class but rather a POSIX program and the protocol between the deployment tool and the provider is based on STDIN/STDOUT/ARGV/ENV.... They will be discovered by a PATH look-up (and will include some version handshake). Might be a much more powerful model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I was thinking but I wanted to talk about it before writing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both of these are tying declaration to implementation.
More generically, how about just an identifier string? (Could be namespaced a la com.amazonaws.cloudformationstack
or something)
In that case, we can even build tools to statically analyze the drops, without necessarily executing them. In direct opposition is the following:
{
"type": "bash -c '$(which foobar)/../libexec/bin/cfnprovider'"
}
How is any tool going to analyze all the CloudFormation stacks in an Assembly if this is the format of "identifier" they may have to contend with?
A compromise might be found in the following:
{
"type": "com.amazonaws.cloudformation.stack",
"executionHint": "cdk-cloudformation-stack"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value of the URI
style string is that the protocol string allows you to specify a mechanism for accessing (e.g: npm://
means you would need node
to be available & get the support by running npm install <packageName>
). Then the path segment would give info about the package coordinates, and then how to use it. Implementations would support certain protocols and know how to execute. So no bash -c blah
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I was responding to Elad's suggestion of using subprocesses here.
-
Even if we were to use
npm://
coordinates, does the identifier have meaning without the NPM package that contains code to interpret it? To have any hope of tools that interpret the model apart from simply executing it, it must.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree with Rico. Why limit ourselves to a string, when we have the full power of JSON available? I think the Drop Type
being an object is more powerful (and easier to extend in the future).
Property |Type |Required|Description | ||
-------------|--------------------|:------:|----------- | ||
`template` |`string` |Required|The assembly-relative path to the *CloudFormation* template document. | ||
`parameters` |`Map<string,string>`| |Parameters to be passed to the [stack][CFN Stack] upon deployment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the only way to order drops by using the output
of one in the parameters
of another? We have more need of sequencing stack drops that aren't necessarily expressed as a threading of information from one object to the other (yay, side effects).
For example, an Export
in one stack and an Import
in the other.
I would argue for a field similar to CloudFormation's DependsOn
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Export/Import is a good case. Although one could say you might want to use Parameters to make the dependency explicit (but you certainly don't have to). I'm adding an optional dependsOn
array.
specifications/cloud_assembly.md
Outdated
Property |Type |Required|Description | ||
------------|--------|:------:|----------- | ||
`savedImage`|`string`|Required|The assembly-relative path to the tar archive obtained from `docker image save`. | ||
`imageName` |`string`|Required|The name of the image (e.g: `000000000000.dkr.ecr.bermuda-triangle-1.amazon.com/name`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer for this name to be something like pushTarget
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah these serve mostly as example, so names in here are definitely non-normative and I've been... favoring speed over quality :D
This adds a rule to make sure the codebase is all configured to support node runtimes complying with `>= 8.11.0`, by ensuring that the configured `@types/node` version starts with `^8.11.`. In order to allow applying this rule to the top-level `package.json`, a feature was added to `pkglint` that allows users to confgure `include` and `exclude` filters as part of their `package.json`'s `pkglint` setup. Exclusions always have precedence over inclusions, and specifying `disable: true` has the same effect as specifying `exclude: '*'`. Both the `include` and `exclude` values can contain patterns that use `*` as a wild-card that maches any number of characters (including no character at all), and can be either a single pattern, or an array of patterns. `include` defaults to `*`.
|
||
In other words, *Logical IDs* are expected to match the following regular expression: | ||
```js | ||
/^[A-Za-z0-9+\/_-]{1,256}$/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be things like -
, +
, _
, 0
then? Seems a little too permissive maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? I have no problem with those. Their primary goal is not human-readability, it's supposed to be machine-processed. I find expressing "only letters and numbers, -, +, _, but must start by a letter" starts looking like the expression of a poor password policy... I don't want to impose restrictions that are convoluted to express in plain English unless I have a good reason to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think 0
, or -
, or +
are great logical IDs, but maybe it's just me. Feel free to ignore.
specifications/cloud_assembly.md
Outdated
Key |Type |Required|Description | ||
-------------|----------------------|:------:|----------- | ||
`type` |`string` |Required|The [*Drop Type*](#drop-type) specifier of this Drop. | ||
`environment`|`string` |required|The target [environment](#environment) in which Drop is deployed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"in which this Drop is deployed"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great :). Some minor comments.
specifications/cloud_assembly.md
Outdated
`metadata` |`Map<string,Metadata>`| |Arbitrary named [metadata](#metadata) associated with this Drop. | ||
`properties` |`Map<string,any>` | |The properties of this Drop as documented by its maintainers. | ||
|
||
Each [Drop Type](#drop-type) can produce output strings that allow Drops to provide informations that other Drops can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this paragraph, and the ones below, deserve a separate section. They seem a little too deep after just being introduced to what drops are for the first time.
└───────────── The Logical ID of the Drop | ||
``` | ||
|
||
The following escape sequences are valid: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are the escape sequences valid? All strings everywhere in the cloud assembly? Keys, values, both? Would be good to clarify I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. They're only meaningful in places where the interpolation is (aka - values of the properties
map).
Deployment systems **MUST** support at least one protocol, and **SHOULD** support all the protocols specified in | ||
the following sub-sections. | ||
|
||
##### The `npm` protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree with Rico. Why limit ourselves to a string, when we have the full power of JSON available? I think the Drop Type
being an object is more powerful (and easier to extend in the future).
╰┬╯ ╰──┬──╯ ╰──┬─╯ | ||
│ │ └─ The name of an AWS region (e.g: eu-west-1) | ||
│ └───────── An AWS account ID (e.g: 123456789012) | ||
└───────────────── AWS protocol specifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This already seems not powerful enough (cross-region CodePipelines? Cross-account CodePipelines?).
Again, I would suggest using more of JSON's power here than just a string
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A Drop
lives in exactly one account/region. Cross-region / cross-account CodePipelines will involve multiple Drop
s in different environments.
└───────────────── AWS protocol specifier | ||
``` | ||
|
||
### Metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, is metadata an object with just 2 keys? Seems quite limiting... what if I have a bunch of warnings/errors for the same file, seems like putting them in a list would be convenient... I would think about this a little more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. I came to realize this as I was trying to implement... :)
What's going on with this PR? |
Hold it off for now. We are meeting with CNAB next week and will be wiser after the meeting. |
Abandoning this for now (resolving without merge). |
What will be the future direction regarding the Cloudlet/CNAB topic ? |
@hjander thanks for querying - @RomainMuller left a comment on #956 -- shall we continue the discussion there? |
Looking forward to supporting a more integrated deployment experience
with support of multiple stacks (with deployment order constraints
honored) as well as assets, this proposes a specification for a
Cloud Assembly that determines a document storage format.
Fixes: #956
TODO: