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

Fix generated package and class names for versioned APIs #381

Closed
Tracked by #598
pawelprazak opened this issue Apr 13, 2022 · 11 comments · Fixed by pulumi/pulumi-kubernetes#2055 or #719
Closed
Tracked by #598

Fix generated package and class names for versioned APIs #381

pawelprazak opened this issue Apr 13, 2022 · 11 comments · Fixed by pulumi/pulumi-kubernetes#2055 or #719
Assignees
Labels
impact/breaking Fixing this issue will require a breaking change impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).
Milestone

Comments

@pawelprazak
Copy link
Contributor

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Currently when generating providers we don't handle versioned APIs very well (kubernetes, google-native, others?).

Expectations:

  • io.pulumi.googlenative.accesscontextmanager_v1 -> io.pulumi.googlenative.accesscontextmanager.v1
  • Accesscontextmanager_v1Functions -> AccesscontextmanagerFunctions

I've seen this mechanism used to correct this type of provider specific problems:
VirtusLab/pulumi-kubernetes@2e08c5e#diff-8dfd74ae4dfdfd04f5c226c8e65a2d3060d6761b45252ba7a757c22492d99311R42460

Underscores in packages should only be used to escape reserved keywords and when the name starts with a number, e.g.:

  • org.example.foo.class_.for_.void_
  • org/example.bar._23games

Affected area/feature

@pawelprazak pawelprazak added kind/enhancement Improvements or new features impact/breaking Fixing this issue will require a breaking change impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec size/S Estimated effort to complete (1-2 days). labels Apr 13, 2022
@mikhailshilkov mikhailshilkov added kind/enhancement Improvements or new features and removed kind/bug Some behavior is incorrect or out of spec kind/enhancement Improvements or new features labels May 5, 2022
@pawelprazak
Copy link
Contributor Author

I was thinking about adding a section to pulumi-java-gen.yaml something like:

schemaOverlay:
    language:
        packages:
            admissionregistration.k8s.io/v1: "admissionregistration.v1"
            ....

To give ability of augmenting the schema with additional information instead of forcing language specific changes into the schema.

@t0yv0 WDYT?

@t0yv0
Copy link
Member

t0yv0 commented May 16, 2022

What does the C# projection do and how does it do it?

Found this:

https://github.com/pulumi/pulumi-google-native/blob/351aaf086ff1e630cda07b54a7f623dce8c4a6db/sdk/dotnet/AccessContextManager/V1/Inputs/DevicePolicyArgs.cs

VS this:

https://github.com/pulumi/pulumi-java/tree/main/providers/pulumi-google-native/sdk/java/src/main/java/com/pulumi/googlenative/accesscontextmanager_v1

C# has this language-specific config:

anton@Antons-MacBook-Pro> jq .language.csharp ~/schemas/pulumi-kubernetes-v3.15.1.json

{
  "compatibility": "kubernetes20",
  "dictionaryConstructors": true,
  "namespaces": {
    "": "Provider",
    "admissionregistration.k8s.io/v1": "AdmissionRegistration.V1",
    "admissionregistration.k8s.io/v1beta1": "AdmissionRegistration.V1Beta1",
    "apiextensions": "ApiExtensions",
    "apiextensions.k8s.io/v1": "ApiExtensions.V1",
    "apiextensions.k8s.io/v1beta1": "ApiExtensions.V1Beta1",
    "apiregistration.k8s.io/v1": "ApiRegistration.V1",
    "apiregistration.k8s.io/v1beta1": "ApiRegistration.V1Beta1",
    "apps/v1": "Apps.V1",
    "apps/v1beta1": "Apps.V1Beta1",
    "apps/v1beta2": "Apps.V1Beta2",
    "auditregistration.k8s.io/v1alpha1": "AuditRegistraion.V1Alpha1",
    "authentication.k8s.io/v1": "Authentication.V1",
    "authentication.k8s.io/v1beta1": "Authentication.V1Beta1",
    "authorization.k8s.io/v1": "Authorization.V1",
    "authorization.k8s.io/v1beta1": "Authorization.V1Beta1",
    "autoscaling/v1": "Autoscaling.V1",
    "autoscaling/v2": "Autoscaling.V2",
    "autoscaling/v2beta1": "Autoscaling.V2Beta1",
    "autoscaling/v2beta2": "Autoscaling.V2Beta2",
    "batch/v1": "Batch.V1",
    "batch/v1beta1": "Batch.V1Beta1",
    "batch/v2alpha1": "Batch.V2Alpha1",
    "certificates.k8s.io/v1": "Certificates.V1",
    "certificates.k8s.io/v1beta1": "Certificates.V1Beta1",
    "coordination.k8s.io/v1": "Coordination.V1",
    "coordination.k8s.io/v1beta1": "Coordination.V1Beta1",
    "core/v1": "Core.V1",
    "discovery.k8s.io/v1": "Discovery.V1",
    "discovery.k8s.io/v1beta1": "Discovery.V1Beta1",
    "events.k8s.io/v1": "Events.V1",
    "events.k8s.io/v1beta1": "Events.V1Beta1",
    "extensions/v1beta1": "Extensions.V1Beta1",
    "flowcontrol.apiserver.k8s.io/v1alpha1": "FlowControl.V1Alpha1",
    "flowcontrol.apiserver.k8s.io/v1beta1": "FlowControl.V1Beta1",
    "flowcontrol.apiserver.k8s.io/v1beta2": "FlowControl.V1Beta2",
    "helm.sh/v2": "Helm.V2",
    "helm.sh/v3": "Helm.V3",
    "meta/v1": "Meta.V1",
    "networking.k8s.io/v1": "Networking.V1",
    "networking.k8s.io/v1beta1": "Networking.V1Beta1",
    "node.k8s.io/v1": "Node.V1",
    "node.k8s.io/v1alpha1": "Node.V1Alpha1",
    "node.k8s.io/v1beta1": "Node.V1Beta1",
    "pkg/version": "Pkg.Version",
    "policy/v1": "Policy.V1",
    "policy/v1beta1": "Policy.V1Beta1",
    "rbac.authorization.k8s.io/v1": "Rbac.V1",
    "rbac.authorization.k8s.io/v1alpha1": "Rbac.V1Alpha1",
    "rbac.authorization.k8s.io/v1beta1": "Rbac.V1Beta1",
    "scheduling.k8s.io/v1": "Scheduling.V1",
    "scheduling.k8s.io/v1alpha1": "Scheduling.V1Alpha1",
    "scheduling.k8s.io/v1beta1": "Scheduling.V1Beta1",
    "settings.k8s.io/v1alpha1": "Settings.V1Alpha1",
    "storage.k8s.io/v1": "Storage.V1",
    "storage.k8s.io/v1alpha1": "Storage.V1Alpha1",
    "storage.k8s.io/v1beta1": "Storage.V1Beta1",
    "yaml": "Yaml"
  },
  "packageReferences": {
    "Glob": "1.1.5",
    "Pulumi": "3.*"
  }
}

@t0yv0
Copy link
Member

t0yv0 commented May 16, 2022

This sounds like a good idea to me based on the above evidence, but I'd welcome input from providers team also perhaps @jkodroff how important this is especially prioritizing against our other gaps.

@t0yv0
Copy link
Member

t0yv0 commented May 16, 2022

I was thinking about adding a section to pulumi-java-gen.yaml something like:

pulumi-java-gen.yaml will be going away, but we do have the .language section in schema with language-specific options; this has the PackageInfo model in the Go code. This would be a perfect extension point for what we need here.

@pawelprazak
Copy link
Contributor Author

I was thinking about adding a section to pulumi-java-gen.yaml something like:

pulumi-java-gen.yaml will be going away, but we do have the .language section in schema with language-specific options; this has the PackageInfo model in the Go code. This would be a perfect extension point for what we need here.

Hmm, interesting, I thought it was a very good direction to decouple the lang specific parts from lang-agnostic parts.
This is very important for any community efforts to add any new language.
I don't expect Pulumi to be interested in maintaining every single language on the planet ;)
and I assume there are passionate people in many communities that would be happy do develop and maintain their favorite language implementation.

With that in mind I always viewed the Java's project setup and all of the various build mechanisms to be an effort in that direction.

IMHO, having language specific information mixed with language agnostic information is not clean design.

@t0yv0
Copy link
Member

t0yv0 commented Jun 2, 2022

I think we need to tackle this.

@t0yv0 t0yv0 mentioned this issue Jun 2, 2022
40 tasks
@t0yv0 t0yv0 self-assigned this Jun 7, 2022
@t0yv0
Copy link
Member

t0yv0 commented Jun 7, 2022

@pawelprazak if you have WIP here that implements this could you link it here?

@t0yv0 t0yv0 added this to the 0.74 milestone Jun 7, 2022
@pawelprazak
Copy link
Contributor Author

pawelprazak commented Jun 9, 2022

Not a WIP but a patch I've used during PoC. Might be helpful.
pulumi/pulumi-kubernetes@2e08c5e#diff-8dfd74ae4dfdfd04f5c226c8e65a2d3060d6761b45252ba7a757c22492d99311R42460

@pawelprazak pawelprazak self-assigned this Jun 29, 2022
pawelprazak added a commit to pawelprazak/pulumi-kubernetes that referenced this issue Jul 1, 2022
pawelprazak added a commit that referenced this issue Jul 1, 2022
- temporary patch for schema until upstream PR is merged

Fixes #381
pawelprazak added a commit that referenced this issue Jul 1, 2022
@pawelprazak
Copy link
Contributor Author

pawelprazak commented Jul 1, 2022

To completely address this issue, there will be several steps required:

pawelprazak added a commit that referenced this issue Jul 1, 2022
- temporary patch for schema until upstream PR is merged

Fixes #381
pawelprazak added a commit that referenced this issue Jul 1, 2022
pawelprazak added a commit to pawelprazak/pulumi-kubernetes that referenced this issue Jul 11, 2022
pawelprazak added a commit to pawelprazak/pulumi-google-native that referenced this issue Jul 11, 2022
pawelprazak added a commit to pawelprazak/pulumi-google-native that referenced this issue Jul 11, 2022
pawelprazak added a commit to pawelprazak/pulumi-google-native that referenced this issue Jul 11, 2022
pawelprazak added a commit to pawelprazak/pulumi-kubernetes that referenced this issue Jul 11, 2022
pawelprazak added a commit that referenced this issue Jul 11, 2022
- temporary patch for schema until upstream PR is merged

Fixes #381
pawelprazak added a commit that referenced this issue Jul 11, 2022
pawelprazak added a commit that referenced this issue Jul 11, 2022
- temporary patch for schema until upstream PR is merged

Part of #381
pawelprazak added a commit that referenced this issue Jul 11, 2022
@pawelprazak pawelprazak reopened this Jul 12, 2022
pawelprazak added a commit that referenced this issue Jul 12, 2022
- temporary patch for schema until upstream PR is released

Part of #381
pawelprazak added a commit that referenced this issue Jul 12, 2022
pawelprazak added a commit that referenced this issue Jul 12, 2022
- temporary patch for schema until upstream PR is merged

Part of #381
pawelprazak added a commit that referenced this issue Jul 12, 2022
pawelprazak added a commit that referenced this issue Jul 12, 2022
- temporary patch for schema until upstream PR is merged

Part of #381
pawelprazak added a commit that referenced this issue Jul 12, 2022
@pawelprazak pawelprazak reopened this Jul 12, 2022
@t0yv0
Copy link
Member

t0yv0 commented Jul 12, 2022

I've taken follow up items into #732 so we can close this as done.

@t0yv0 t0yv0 closed this as completed Jul 12, 2022
@t0yv0 t0yv0 mentioned this issue Jul 12, 2022
3 tasks
mikhailshilkov pushed a commit to pulumi/pulumi-google-native that referenced this issue Jul 13, 2022
pawelprazak added a commit to pawelprazak/pulumi-azure-native that referenced this issue Jul 15, 2022
pawelprazak added a commit to pawelprazak/pulumi-azure-native that referenced this issue Jul 15, 2022
stack72 pushed a commit to pulumi/pulumi-azure-native that referenced this issue Aug 12, 2022
stack72 pushed a commit to pulumi/pulumi-azure-native that referenced this issue Aug 12, 2022
stack72 pushed a commit to pulumi/pulumi-azure-native that referenced this issue Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/breaking Fixing this issue will require a breaking change impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).
Projects
None yet
4 participants