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

[cli] Import of resources without a validation creates an unusable python output #33

Closed
iliapolo opened this issue Sep 2, 2020 · 8 comments
Assignees

Comments

@iliapolo
Copy link
Member

iliapolo commented Sep 2, 2020

Description of the bug:

When a CRD has no validation, the generated python class does not allow passing in any options, meaning you can't configure it.

Reproduction Steps:

Save the following CRD in a file:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: widgets.com.foo.bar
spec:
  group: com.foo.bar
  names:
    kind: Widget
    listKind: Widgets
    plural: widgets
    singular: widget
  scope: Namespaced
  subresources:
    status: {}
  version: v1
  versions:
  - name: v1
    served: true
    storage: true

and run cdk8s import --language python <file>

The generated python class will look like so:

class Widget(cdk8s.ApiObject, metaclass=jsii.JSIIMeta, jsii_type="generated.Widget"):
    """
    schema:
    :schema:: Widget
    """

    def __init__(self, scope: constructs.Construct, name: str) -> None:
        """Defines a "Widget" API object.

        :param scope: the scope in which to define this object.
        :param name: a scope-local name for the object.
        """
        options = WidgetOptions()

        jsii.create(Widget, self, [scope, name, options])

Notice that the __init__ method does not accept any options.

Alternatively, when generating typescript code, the Widget class accepts an empty interface, allowing the user to pass any specification, which is the expected behavior.

/**
 *
 *
 * @schema Widget
 */
export class Widget extends ApiObject {
  /**
   * Defines a "Widget" API object
   * @param scope the scope in which to define this object
   * @param name a scope-local name for the object
   * @param options configuration options
   */
  public constructor(scope: Construct, name: string, options: WidgetOptions = {}) {
    super(scope, name, {
      ...options,
      kind: 'Widget',
      apiVersion: 'com.foo.bar/v1',
    });
  }
}

/**
 * @schema Widget
 */
export interface WidgetOptions {
}

Error Log:

No error, the generated code is unusable.

Environment:

  • Framework Version: 0.26.0
  • OS: Any

Other:


This is 🐛 Bug Report

@iliapolo iliapolo self-assigned this Sep 2, 2020
@iliapolo iliapolo transferred this issue from cdk8s-team/cdk8s Jul 6, 2021
@Chriscbr
Copy link
Contributor

Ha - so it looks like this issue should also be fixed by #44. 😅 By adding "metadata" to all imported CRDs regardless, the generated python class constructors will always have some options you can pass. Here's a snapshot that gives an idea of how the changes in #44 affect the jsii output in Python from my editor:

Screen Shot 2021-07-24 at 7 44 18 PM

The underlying jsii edge case / bug still exists, but it shouldn't affect imports.

@iliapolo
Copy link
Member Author

@Chriscbr

By adding "metadata" to all imported CRDs regardless, the generated python class constructors will always have some options you can pass

Yes but - will this allow users to pass in arbitrary specs? or just metadata..?

@Chriscbr
Copy link
Contributor

Chriscbr commented Jul 26, 2021

Ah I see - if there is no "validation" needed, then we simply want it to allow anything (including both metadata and spec information). This makes sense to me now. 👍

@Chriscbr
Copy link
Contributor

I'm taking a closer look now, and it seems like if we want to support arbitrary properties, we probably want to model it in the way ApiObjectMetadata specifies arbitrary fields:

https://github.com/cdk8s-team/cdk8s-core/blob/c169366ec31b55a164824769b4295b005d92269a/src/metadata.ts#L55-L58

And this too is not correctly supported in the Python transliteration, so it looks like in general this is a broader jsii issue.

@github-actions
Copy link
Contributor

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

@github-actions github-actions bot added the stale label Sep 25, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2021

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.

@github-actions github-actions bot closed this as completed Oct 3, 2021
@Chriscbr Chriscbr reopened this Oct 5, 2021
@Chriscbr Chriscbr removed the stale label Oct 5, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2021

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

@github-actions github-actions bot added the stale label Dec 5, 2021
@github-actions
Copy link
Contributor

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen.

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

No branches or pull requests

2 participants