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

add yaml struct tags for generated golang structs #3293

Closed
2 of 7 tasks
Hunter-Thompson opened this issue Dec 24, 2021 · 5 comments · Fixed by #3299
Closed
2 of 7 tasks

add yaml struct tags for generated golang structs #3293

Hunter-Thompson opened this issue Dec 24, 2021 · 5 comments · Fixed by #3299
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@Hunter-Thompson
Copy link
Contributor

Hunter-Thompson commented Dec 24, 2021

🚀 Feature Request

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)
  • Go

General Information

  • JSII Version: 1.46.0

  • Platform: N/A

  • I may be able to implement this feature request

  • This feature might incur a breaking change

Description

In Go, JSII generates structs with json struct tags :

type ProviderConfig struct {
    Data *string `json:"data"`
}

It would be nice to have yaml struct tags too. While marshaling/unmarshaling this would help.

Proposed Solution

type ProviderConfig struct {
    Data *string `json:"data" yaml:"data"`
}

Notes

If you could point me to the file which does this I may be able to write a PR.

@Hunter-Thompson Hunter-Thompson added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 24, 2021
@MrArnoldPalmer
Copy link
Contributor

I see no problem with this, but I'm wondering what your use case is? The JSON struct tags are generated to help marshaling/demarshalling as JSII types are serialized to JSON when passed to the kernel via stdin, and deserialized when received from the kernel via stdout.

Generally the struct tags aren't meant to be seen by consumers of generated JSII Go modules and are an implementation detail, though there may be some uses I'm not considering.

@Hunter-Thompson
Copy link
Contributor Author

Hunter-Thompson commented Dec 25, 2021

Generally the struct tags aren't meant to be seen by consumers of generated JSII Go modules and are an implementation detail, though there may be some uses I'm not considering.

When we create a construct library in cdk8s using TS, since the generated structs in golang only have JSON struct tags, I cannot use those structs to marshal YAML data into it.

Lets look at an interface:

export interface CustomInterface {
    readonly config: Config;
}

export interface Config {
    readonly containerName: string;
    readonly containerImage: string
}

The generated structs for this would be :

type struct CustomInterface {
    Config Config `json:"config"`
}

type struct Config {
    ContainerName string `json:"containerName"`
    ContainerImage string `json:"containerImage"`
}

The application I'm building marshalls data into the CustomInterface struct from a YAML config which would look like this for our CustomInterface:

config:
  containerName: test
  containerImage: test-image:latest

Then it takes those values and then creates a new container definition in cdk8s.

podSpec := &k8s.PodSpec{
    Containers: &[]*k8s.Container{
        {
            Name:  jsii.String(config.containerName),
            Image: jsii.String(config.containerImage),
        },
    },
}

Sort of like helm but without the madness : )

The problem here is that the camelCase is not being respected by the Marshaller, thats why yaml struct tags are necessary. I hope I was able to convey my use case.

@MrArnoldPalmer
Copy link
Contributor

gotcha, so your application accepts some yaml configuration loaded and deserialized and passed as args to cdk8s constructs? Seems straightforward enough and I don't see any downside to enabling easier yaml deserialization. 👍🏻

@Hunter-Thompson
Copy link
Contributor Author

Hunter-Thompson commented Dec 26, 2021

gotcha, so your application accepts some yaml configuration loaded and deserialized and passed as args to cdk8s constructs?

yep, you nailed it.

@mergify mergify bot closed this as completed in #3299 Dec 27, 2021
mergify bot pushed a commit that referenced this issue Dec 27, 2021
closes #3293 

---

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

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants