fix: include namespace in renderedYaml filename

[rawkode]

This fixes a problem when resources have the same name/kind, but live in different namespaces. This caused the first file written to be overwritten by the later resources.

[rawkode]

@lblackstone I'm not sure if GetNamespace() will work on resources without a namespace. I'm going to run some tests now :)

[rawkode]

The test that failed seems unrelated 👍

[lblackstone]

/run-acceptance-tests

[github-actions]

Please view the PR build - https://github.com/pulumi/pulumi-kubernetes/actions/runs/497210330

[lblackstone]

Thanks for the PR!

I do have a couple concerns about this change so far:

1. While changing the name does resolve the bug, it also would rename every generated resource YAML, which might break users' existing workflows. Adding the namespace is a good way to disambiguate, but it would be ideal if we could leave existing stacks alone. I'm not sure off the top of my head how easy that would be to check.
2. GetNamespace() will return "" for the default namespace, so it would be better to format the string to avoid the extra - in that case. e.g. pod-foo rather than pod--foo.

[rawkode]

@lblackstone With regards to issue 1 - I don't think that's a problem. Anyone generating the YAML will be using kubectl to apply and the filenames will be irrelevant. The directories created won't be modified, which I believe is safe to assume that's as close to coupling a consumer to renderToYaml may get. IMO.

Happy to implement issue 2 if you're happy to proceed on the above hypothesis.

Option 3 could be to add another option to the provider config to tweak the filenames, but I don't think that would be used really?

[lblackstone]

Anyone generating the YAML will be using kubectl to apply and the filenames will be irrelevant. The directories created won't be modified, which I believe is safe to assume that's as close to coupling a consumer to renderToYaml may get...Happy to implement issue 2 if you're happy to proceed on the above hypothesis.

Yeah, that's probably a reasonable assumption. Let's proceed with this change, and I'll bump the minor version of the next release since it's technically a breaking change.

[leezen]

@rawkode Just checking in to see if you plan to come back to this. Thanks!

[rawkode]

@rawkode Just checking in to see if you plan to come back to this. Thanks!

The last message from @lblackstone said they where happy to go with it as it is, under our assumptions.

I was just waiting for this to be merged.

[lblackstone]

Oh, sorry for the confusion. There's still one point of feedback to address, and then we can merge.

GetNamespace() will return "" for the default namespace, so it would be better to format the string to avoid the extra - in that case. e.g. pod-foo rather than pod--foo.

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[lblackstone]

/run-acceptance-tests

[github-actions]

Please view the PR build - https://github.com/pulumi/pulumi-kubernetes/actions/runs/603420812

[lblackstone]

Thanks for the PR!