Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

CRD Generation #4

Merged
merged 16 commits into from
Aug 19, 2021
Merged

CRD Generation #4

merged 16 commits into from
Aug 19, 2021

Conversation

muvaf
Copy link
Member

@muvaf muvaf commented Aug 12, 2021

We are able to generate any CRD from given Terraform resource. An example usage of these utilities are in crossplane-contrib/provider-jet-aws#3 . There are missing pieces but this will serve as a starting point.

Note that I have used some utilities from https://github.com/muvaf/typewriter . If we don't want to depend on that repo, we can reimplement those functionalities in the next iteration.

Signed-off-by: Muvaffak Onus <me@muvaf.com>
… or given by the user

Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
since that prevents multiple crds from being generated at once

Signed-off-by: Muvaffak Onus <me@muvaf.com>
@muvaf muvaf force-pushed the builder branch 2 times, most recently from dcc8bed to 35ee0e8 Compare August 12, 2021 18:33
Signed-off-by: Muvaffak Onus <me@muvaf.com>
…remove doc file since its content needs to be in zz_types anyway.

Signed-off-by: Muvaffak Onus <me@muvaf.com>
… cases

Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
@muvaf
Copy link
Member Author

muvaf commented Aug 18, 2021

Note that I had to put every CRD in its own package because the types originally don't have a name in the schema (they are just maps with some fields) and we produce a name for them like FieldNameParameters and FieldNameObservation, however, this results in collisions between the types. I tried to prefix it with the path in the recursion tree but that resulted in VERY LONG type names and there was still possibility of collision. So I opted for putting each CRD into its own world and let it import the scheme and other stuff from version package.

Copy link
Member

@turkenh turkenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, added a couple of comments which are mostly nits.

pkg/pipeline/group.go Outdated Show resolved Hide resolved
pkg/pipeline/group.go Outdated Show resolved Hide resolved
pkg/pipeline/group.go Outdated Show resolved Hide resolved
pkg/pipeline/templates/crd_types.go.tmpl Show resolved Hide resolved
pkg/types/builder.go Outdated Show resolved Hide resolved

obsName := types.NewTypeName(token.NoPos, g.Package, obsTypeName, nil)
obsType = types.NewNamed(obsName, types.NewStruct(obsFields, obsTags), nil)
g.genTypes[obsType.Obj().Name()] = obsType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
g.genTypes[obsType.Obj().Name()] = obsType
g.genTypes[obsTypeName] = obsType

var paramType, obsType *types.Named
paramName := types.NewTypeName(token.NoPos, g.Package, paramTypeName, nil)
paramType = types.NewNamed(paramName, types.NewStruct(paramFields, paramTags), nil)
g.genTypes[paramType.Obj().Name()] = paramType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
g.genTypes[paramType.Obj().Name()] = paramType
g.genTypes[paramTypeName] = paramType

switch sch.Type {
case schema.TypeMap:
return types.NewMap(types.Universe.Lookup("string").Type(), elemType), nil
default:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we convert both schema.TypeList and schema.TypeSet to slice on our side.
I am just wondering whether this would cause any issues like expecting ordered items and causing diffs because of that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say I'm 100% sure but I believe these two types correspond to same thing (array) but they are accounted for during a diff operation. My hunch is that the separation is important for logic below terraform cli but it's worth testing after we get all PRs to end-to-end working state.

Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
@muvaf muvaf merged commit 48f7e68 into crossplane:main Aug 19, 2021
@muvaf muvaf deleted the builder branch August 19, 2021 12:46
@muvaf muvaf mentioned this pull request Aug 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants