search

aws-cloudformation / aws-cloudformation-resource-providers-cloudformation

The CloudFormation Resource Provider Package For AWS CloudFormation
https://aws.amazon.com/cloudformation/
Apache License 2.0
48 stars 35 forks source link

AWS::CloudFormation::Type #3

Open pgarbe opened 5 years ago

pgarbe commented 5 years ago

1. Title

AWS::CloudFormation::Type

2. Scope of request

AWS::CloudFormation::Type - can create resource via API, but not via CloudFormation

3. Expected behavior

In Create, the RegisterType should be registered, and in Delete it should be unregistered. It seems that updates are not supported.

4. Suggest specific test cases

Just being able to do the same stuff as with the API / CLI.

5. Helpful Links to speed up research and evaluation

See registerType and deregisterType in https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/CloudFormation.html

6. Category (required) - Will help with tagging and be easier to find by other users to +1

  1. Management (CloudTrail, Config...)
rjlohan commented 5 years ago

We are working on this one, we just have some back and forth on the right model here mainly with respect to setting default versions and how that should best work in CloudFormation stack. 👍🏼

benkehoe commented 5 years ago

Why not put that discussion about how default versions should be managed here on the roadmap, and let customers give feedback on the options?

rjlohan commented 5 years ago

So the way we are thinking about this type is that you'd have a model something like below (represented as a CloudFormation template snippet);

Type: AWS::CloudFormation::Type
Properties:
    Kind: Resource
    TypeName: Organisation::Service::Resource
    DefaultVersion: "00000003"
    ExecutionRoleArn: arn:aws:iam::123456789012:role/MyExecutionRole
    SchemaHandlerPackage: my-artifact-bucket
    LoggingConfiguration:
        LogRoleArn: arn:aws:iam::123456789012:role/MyLoggingRole
        LogGroupName: MyResourceLogGroups

The !Ref-able identity of this type would be the TypeArn

Provisioning semantics;

I think that's the gist of the discussion I have had with @aygold92 so far.

pgarbe commented 5 years ago

Can you elaborate a bit about DefaultVersion? What's the intention of it and how should it be handled? Isn't the versioning handled by SchemaHandlerPackage (like having versioned artifacts in that bucket)?

rjlohan commented 5 years ago

After you invoke RegisterType a new TypeArn with new version is created. However you must then call the SetTypeDefaultVersion API to make that version the one which will be used by CloudFormation in provisioning operations. See the docs for more explanation: https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_SetTypeDefaultVersion.html

pgarbe commented 5 years ago

I think I got it. So the DefaultVersion could point to a different (previous) version than the version that's provided in SchemaHandlerPackage, right? This would be very confusing.

What about skipping the DefaultVersion completely (in the Type resource)? Updates would lead to a new registration, setting this version as default, and deregistration of the old one.

benkehoe commented 5 years ago

A few questions for @rjlohan about this.

rjlohan commented 5 years ago

OK, here's an idea. Why not help us build it? 😁

I'm going to move this issue to a more appropriate place, and I'm willing to take on feedback and contributions to see what we can come up with together.

image

rjlohan commented 5 years ago
  • Why is Kind a property? Why not have the resource type be AWS::CloudFormation::Resource? I get that the API is RegisterType with a Type field in it, but I think it's better to reify them into proper CloudFormation types than have this weird subtyping going on.

Right now the Registry only supports the ability to (De)RegisterType, however we designed it intentionally to be as agnostic of CloudFormation (the service) as we could. Certainly there will be abstraction leaks, but we intend to separate concerns so that we have the ability to cleanly build out other capabilities in the future. I'm OK with modelling the current RESOURCE kind as a discrete entity, though the intent is for the registration process to not really care that the end-user is modelling a CloudFormation Resource or some other entity.

For now, we have a rolling, immutable versioned history of the evolution of a Type. I don't believe this to be a one-way door so that we can introduce semantic versioning/labels later. We're not there yet.

  • You should require DeletionPolicy: Retain on these resources, since they create versions but do not delete them.

That would be a leaky abstraction. The Resource platform does not know anything about CloudFormation's stack-level concerns. (To be fair a couple of things leaked through, but again the intent was to design the Registry and the resource platform to be completely oblivious to CloudFormation the service.

  • I agree with @pgarbe that it's strange to have the DefaultVersion on the resource representing a different version from the version that the resource itself represents. I think there are two non-exclusive options:

    • There's a property on the resource that enables the "update default version" behavior
    • There's a separate resource type for aliases, of which there's only the "default" today.
  • This is an API question, but the SchemaHandlerPackage is just a bucket, not a location within a bucket? What happens to my old versions code that I upload? Can I use a bucket that I use for other kinds of code artifacts?

Yeah this is the one bit I don't have a good canned answer on. Vote?

  • The RegisterType API is essentially an upsert...which the resource lifecycle is not supposed to support.

Yes, we'll have to model this correctly in the CreateHandler to ensure a type does not already exist with the TypeName and in the UpdateHandler to ensure the type exists to be updated. The laws of API do not apply to the CloudFormation Registry and the CLI's Resources and vice versa. :)

benbridts commented 4 years ago

I have some thoughts to add:

Types and Versions should be separate resources, because they have separate lifetimes:

Having to specify a Kind seems a bit strange too, I don't think I would mind having a different resource for each (future) kind. Assuming this is expected to be a limited list. I'm going to write the rest of this comment with Kind as a Property, but I think there is an argument to be made for making it a resource level distinction (similar to how API-GW has different resources for different kinds of APIs and ELB has ALB/NLB as different resources).

This could be a way to address versioning:

MyTypeVersion:
    Type: AWS::CloudFormation::TypeVersion
    Properties:
        Kind: Resource
        TypeName: Organisation::Service::Resource
        ExecutionRoleArn: arn:aws:iam::123456789012:role/MyExecutionRole
        SchemaHandlerPackage: my-artifact-bucket
        LoggingConfiguration:
            LogRoleArn: arn:aws:iam::123456789012:role/MyLoggingRole
            LogGroupName: MyResourceLogGroups

MyType:
    Type: AWS::CloudFormation::Type
    Properties:
        TypeVersion: !Ref MyTypeVersion
benkehoe commented 4 years ago

A lot has changed, this discussion is happening primarily on pull request #4 that implements (currently) ::ResourceVersion and ::ResourceVersionAlias

wulfmann commented 4 years ago

Does this merge now resolve this issue? https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-cloudformation/pull/4

rjlohan commented 4 years ago

I'll be working on the deployment on the backend and then will update this ticket when it's available. It'll take a couple of days to go through the works.

jarreds commented 3 years ago

Any word on when this is going live? There seem to be a couple issues open, but I'm not sure which one is the official so xposting.

eduardomourar commented 3 years ago

I believe this can be closed now that both types have been released, right? https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/AWS_CloudFormation.html