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(helm-chart): adjust to entrypoint within dockerfile #43956

Closed

Conversation

pudovd
Copy link
Contributor

@pudovd pudovd commented Jul 9, 2024

fix #43669

@CLAassistant
Copy link

CLAassistant commented Jul 9, 2024

CLA assistant check
All committers have signed the CLA.

@pudovd pudovd force-pushed the helm-chart-event-handler-fix branch from fe008a8 to 0280c20 Compare July 9, 2024 14:24
@zmb3 zmb3 requested a review from hugoShaka July 9, 2024 15:40
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hugoShaka @fheinecke Do you know why the entrypoint changed? Was this a side-effect of plugins migration to teleport repository? I think we should fix the entrypoint back rather.

@r0mant r0mant requested review from fheinecke and removed request for klizhentas July 24, 2024 18:56
@fheinecke
Copy link
Contributor

Thanks for the fix @pudovd. I updated this entrypoint for all plugins when we moved the plugins code into this repo a couple of months ago, but I forgot to update this Helm chart.

@r0mant this was an intentional change - I just forgot to update the chart for the event handler specifically.

@zmb3
Copy link
Collaborator

zmb3 commented Jul 24, 2024

@fheinecke this will need to be buddied

@fheinecke
Copy link
Contributor

Hi @pudovd,

Thanks for your contribution. We are currently unable to run our tests on external contributions. As such, I'll be helping you validate and merge your pull request. I've created an internal PR here and the CI checks came up clean.

Please wait for another approver and I'll get your code merged. You will retain authorship of your commits.

@pudovd
Copy link
Contributor Author

pudovd commented Jul 26, 2024

Hi @fheinecke,

Thanks for your concern about my authorship ❤️
But I just wanted to draw attention to the issue, because I hadn't wanted to build custom helm chart.
So, I don't mind if we just close this PR.

@fheinecke
Copy link
Contributor

Hi @pudovd,

Thanks for discovering and fixing the issue. We've merged your fix here to our master branch, and it should be in a v16 and v15 release within the next week. Let me know if you have any questions!

@fheinecke fheinecke closed this Jul 26, 2024
@pudovd pudovd deleted the helm-chart-event-handler-fix branch July 26, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken reference to exec command in the event handler helm chart
6 participants