Skip to content
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

chore(release): 1.0.0-beta.6 #514

Merged
merged 2 commits into from
Jan 21, 2021
Merged

chore(release): 1.0.0-beta.6 #514

merged 2 commits into from
Jan 21, 2021

Conversation

iliapolo
Copy link
Member

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@iliapolo iliapolo requested a review from eladb January 21, 2021 12:49
@mergify
Copy link
Contributor

mergify bot commented Jan 21, 2021

Your pull request will be updated and merged automatically (do not update manually).

@iliapolo
Copy link
Member Author

@eladb The build is failing because the integration test snapshots are now missing the imports of:

  • apiextensions.crossplane.io.ts
  • meta.pkg.crossplane.io.ts
  • pkg.crossplane.io.ts

Apparently this no longer takes affect:

https://github.com/awslabs/cdk8s/blob/a49a6b187875cc0f52fc6ad3261f211cf2416d80/packages/cdk8s-cli/src/import/crd.ts#L127-L129

The CRD's themselves are hosted here: https://doc.crds.dev/raw/github.com/crossplane/[email protected].

And indeed they do not have a top level kind property. It seems like the objects now only include the spec. Do you have any idea? Maybe @prasek can help out?

@iliapolo
Copy link
Member Author

mergify bot pushed a commit that referenced this pull request Jan 21, 2021
That import is currently broken - not because of a change in `cdk8s` but rather a change in the schema of the crds on the site.
This shouldn't be failing our builds and we should reconsider this as a build time dependency.

See #514 (comment)

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@mergify mergify bot merged commit 4ee2837 into master Jan 21, 2021
@mergify mergify bot deleted the bump/1.0.0-beta.6 branch January 21, 2021 15:24
@prasek
Copy link
Contributor

prasek commented Jan 21, 2021

/cc @hasheddan

@hasheddan
Copy link

@iliapolo apologies for the break here. This has to do with the fact that CRDs are now being stored with internal representation, so those metadata fields are stripped off, but all of the same information is technically available. That endpoint could be updated to return with v1 schema if needed. As a side note, all repos are now indexed by https://doc.crds.dev so this should be much more generally useful now :)

@iliapolo
Copy link
Member Author

@hasheddan So all of the objects currently returned by the endpoint will always have the same GVK?

@hasheddan
Copy link

@iliapolo yes that is correct, it will only return kind: CustomResourceDefinition. However, it is the full internal spec right now. I have been talking with @prasek and am thinking of making the raw/ endpoint just return v1 flavor of CRDs though so that will be easier for y'all to parse. For instance, with my changes, you would get something like this again:

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  creationTimestamp: null
  name: configurationrevisions.pkg.crossplane.io
spec:
  group: pkg.crossplane.io
  names:
    categories:
    - crossplane
    kind: ConfigurationRevision
    listKind: ConfigurationRevisionList
    plural: configurationrevisions
    singular: configurationrevision
  scope: Cluster
  versions:
...

I assume this will be most convenient for you? If not, happy to support other variations :)

@iliapolo
Copy link
Member Author

@hasheddan yes that would be great. I also think it would be more aligned with standard k8s resource definitions, making it easier for other tools to consume as well.

Thanks!

@hasheddan
Copy link

@hasheddan
Copy link

Feel free to tag me on anything if you are seeing issues in the future 👍

@iliapolo
Copy link
Member Author

@hasheddan sweet, thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants