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

Allow almost all printable ASCII characters in environment variables #4369

Open
5 of 6 tasks
HirazawaUi opened this issue Dec 20, 2023 · 20 comments
Open
5 of 6 tasks
Assignees
Labels
lead-opted-in Denotes that an issue has been opted in to a release sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node.
Milestone

Comments

@HirazawaUi
Copy link
Contributor

HirazawaUi commented Dec 20, 2023

Enhancement Description

Please keep this description up to date. This will help the Enhancement Team to track the evolution of the enhancement efficiently.

@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Dec 20, 2023
@kikisdeliveryservice
Copy link
Member

@HirazawaUi Can you check and find out which sig will be owning this enhancement and update this issue.

@HirazawaUi
Copy link
Contributor Author

@HirazawaUi Can you check and find out which sig will be owning this enhancement and update this issue.

I think it should be sig-node

@HirazawaUi
Copy link
Contributor Author

/sig node
/sig api-machinery
/cc @liggitt @thockin

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 21, 2023
@thockin
Copy link
Member

thockin commented Jan 24, 2024

Agree sig-node

@thockin thockin added the lead-opted-in Denotes that an issue has been opted in to a release label Jan 29, 2024
@tjons
Copy link

tjons commented Jan 31, 2024

Hello @HirazawaUi 👋, Enhancements team here.

Just checking in as we approach enhancements freeze on Friday, February 9th, 2024 at 02:00 UTC.

This enhancement is targeting for stage alpha for 1.30 (correct me, if otherwise)

Here's where this enhancement currently stands:

  • KEP readme using the latest template has been merged into the k/enhancements repo.
  • KEP status is marked as implementable for latest-milestone: 1.30. KEPs targeting stable will need to be marked as implemented after code PRs are merged and the feature gates are removed.
  • KEP readme has up-to-date graduation criteria
  • KEP has a production readiness review that has been completed and merged into k/enhancements. (For more information on the PRR process, check here).

For this KEP, we would just need to complete the following:

  • Merge the KEP readme into the k/enhancements repo.
  • Complete the PRR review process and merge it into k/enhancements.
  • Mark this KEP as implementable for latest-milestone: 1.30.

The status of this enhancement is marked as at risk for enhancement freeze. Please keep the issue description up-to-date with appropriate stages as well. Thank you!

@HirazawaUi
Copy link
Contributor Author

The status of this enhancement is marked as at risk for enhancement freeze

I will try to finish it.

@thockin thockin changed the title Allow special characters environment variable Allow almost all printable ASCII characters in environment variables Feb 3, 2024
@thockin
Copy link
Member

thockin commented Feb 6, 2024

@tjons
Copy link

tjons commented Feb 8, 2024

@HirazawaUi everything looks good except I'm not sure that the KRR has final approval from @jpbetz - what is the status?

@tjons
Copy link

tjons commented Feb 9, 2024

With all the requirements fulfilled this enhancement is now marked as tracked for the upcoming enhancements freeze 🚀

Actually, @HirazawaUi I was incorrect. This PR is also missing the v1.30 milestone in the Github issue. Therefore, unfortunately, this enhancement did not meet requirements for enhancements freeze.

If you still wish to progress this enhancement in 1.30, please file an exception request. Thanks!

@salehsedghpour
Copy link
Contributor

/milestone clear

@HirazawaUi
Copy link
Contributor Author

This PR is also missing the v1.30 milestone in the Github issue.

It's very sad.
I took a closer look at the exception description and realized that this enhancement doesn't seem to apply to it very well, and there's no reason we have to implement this enhancement in milestone 1.30, so I decided to move forward with it in 1.31.

@HirazawaUi
Copy link
Contributor Author

/milestone v1.31

@k8s-ci-robot
Copy link
Contributor

@HirazawaUi: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone v1.31

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@thockin
Copy link
Member

thockin commented Feb 9, 2024

Hold up.

Release team, you know I love you all and your work is invaluable. But...why would we punish a contributor for something I screwed up? And for that matter, why do we need to set two independent indicators of intention, which literally mean the same thing?

I know we've got tons of enhancements in flight, but this is silly.

TPS_reports

@katcosgrove
Copy link

@thockin Please be kind to my team. They are contributors too, many of them first timers. Both myself and the enhancements lead have broad latitude to extend a little bit of grace in cases where there has clearly been a minor clerical error preventing an enhancement from being included in the release. You only need to ask for it. In this case, an exception would have been granted (you just have to send a short email), and either myself or @salehsedghpour would have approved the request without a formal exception request if contacted directly.

Shadows, who by their nature have little to no experience running any aspect of a release, do not have such latitude and must comply with the guidelines they have. Those guidelines are strict for a reason. As you said, we have quite a lot of enhancements in flight, and we have to minimize opportunities for confusion by making situations like this the exception rather than the rule. If you would like to see this policy changed, we're happy to discuss it at either the mid-cycle or post-cycle retro.

This enhancement is now considered tracked for v1.30.

/milestone v1.30

@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Feb 9, 2024
@thockin
Copy link
Member

thockin commented Feb 9, 2024

I didn't mean to be unkind, my apologies if it came off that way. "Clerical error" is the right formulation of this - thanks for that. The underlying problem - seemingly redundant information being required - has happened a few times, and it's never been clear how to make that go away (but it HAS gotten better, so credit where it's due). Process should serve humans, not vice-versa.

I somewhat dislike the idea of "direct contact" because I don't want it to be about who requested something, but I ACK that sometimes it just is. If possible, I'd be glad to brainstorm ways we could streamline the workflow, from both the POV of an "unreliable approver" and of an "earnest contributor", while still giving release-team the info you all need.

In this case it was clearly me who screwed up, but I am a screwup with a terrible memory.

I appreciate the response.

@katcosgrove
Copy link

It isn't about who requests something; it's about who can approve it. In a world where you instead chose to just reach out to me or the enhancements lead to note that this was kicked for a clerical error, who you are would not have been relevant to our decision to agree that it was clearly a clerical error. The KEP owner could have done it themself and the outcome would have been the same as if you asked me. Another enhancement owner did exactly that a couple of hours ago and was moved back to tracked without issue.

If you want to see this process improved, please attend the retros for this cycle, suggest an alternative, and take the action item.

@HirazawaUi
Copy link
Contributor Author

HirazawaUi commented Feb 10, 2024

Many thanks to @thockin and @katcosgrove for attention and help on this matter, If I could check in advance whether I need to send a full or mini "exception", then might be able to avoid this problem :-)

And after I read the documentation on exception, I was worried that a full 'exception' would bring an extra trouble to everyone, and I can't explain why this enhancement is crucial for this milestone, so I decided to push it in the next milestone.

Sorry for wasting your time focusing on this tiny issue, but thankfully things are back on track.

@Princesso
Copy link

Hello @HirazawaUi , 👋 1.30 Docs Shadow here.
Does this enhancement work planned for 1.30 require any new docs or modifications to existing docs?
If so, please follow the steps here to open a PR against the dev-1.30 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday, February 22nd, 2024 18:00 PDT.
Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release.
Thank you!

@a-mccarthy
Copy link

Hi @HirazawaUi,

👋 from the v1.30 Communications Team! We'd love for you to opt in to write a feature blog about your enhancement!

We encourage blogs for features including, but not limited to: breaking changes, features and changes important to our users, and features that have been in progress for a long time and are graduating.

To opt in, you need to open a Feature Blog placeholder PR against the website repository.
The placeholder PR deadline is 27th February, 2024.
Here's the 1.30 Release Calendar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lead-opted-in Denotes that an issue has been opted in to a release sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
Status: Tracked for Enhancements Freeze
Development

No branches or pull requests

10 participants