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

[helm]: implement support for defining image pull secrets #5854

Merged

Conversation

pkoutsovasilis
Copy link
Contributor

What does this PR do?

This PR that adds support for defining pullImageSecrets support in the chart

Why is it important?

Users mights need to deploy an agent image from a private repository

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool
  • I have added an integration test or an E2E test

Disruptive User Impact

N/A

How to test this PR locally

mage integration:kubernetesMatrix

@pkoutsovasilis pkoutsovasilis added enhancement New feature or request backport-8.x Automated backport to the 8.x branch with mergify backport-8.16 Automated backport with mergify labels Oct 25, 2024
@pkoutsovasilis pkoutsovasilis self-assigned this Oct 25, 2024
@pkoutsovasilis pkoutsovasilis requested a review from a team as a code owner October 25, 2024 12:40
@pkoutsovasilis pkoutsovasilis changed the title feat: implement support for defining image pull secrets [helm]: implement support for defining image pull secrets Oct 25, 2024
@ycombinator ycombinator requested review from swiatekm and removed request for kaanyalti October 25, 2024 13:55
@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label Oct 25, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Contributor

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

I think it's more idiomatic to attach these to a ServiceAccount instead of the Pod, but in this case it doesn't matter too much.

Also, can you actually set this in one of the examples?

@pkoutsovasilis
Copy link
Contributor Author

I think it's more idiomatic to attach these to a ServiceAccount instead of the Pod, but in this case it doesn't matter too much.

I don't have a preference of one way (ServiceAccount attachment) over the other (Pod attachment), but since there is this draft PR (planning on working on that soon) that will allow the users to complete override service accounts I chose the latter to deterministically always apply the imagePullSecrets and not allow it to be overidden.

Also, can you actually set this in one of the examples?

I can mention it but having it actually in the example cli commands I don't think so as I don't have a guaranteed elastic-agent image under a private registry and I would like to keep the examples as simple as a straight-forward copy-paste. Would that work or you had something else in mind?

@swiatekm
Copy link
Contributor

I can mention it but having it actually in the example cli commands I don't think so as I don't have a guaranteed elastic-agent image under a private registry and I would like to keep the examples as simple as a straight-forward copy-paste. Would that work or you had something else in mind?

That's fine. I mostly just want a sanity check that the template works as expected, but for a change as simple as this one it's not that important.

@pkoutsovasilis pkoutsovasilis force-pushed the feat/helm_chart_image_pull_secrets branch from f36b847 to 4d77360 Compare October 30, 2024 09:57
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@pkoutsovasilis pkoutsovasilis merged commit 053b320 into elastic:main Oct 30, 2024
9 checks passed
mergify bot pushed a commit that referenced this pull request Oct 30, 2024
* feat: implement support for defining image pull secrets

* doc: add imagePullSecrets in kubernetes default example

(cherry picked from commit 053b320)

# Conflicts:
#	deploy/helm/elastic-agent/values.yaml
mergify bot pushed a commit that referenced this pull request Oct 30, 2024
* feat: implement support for defining image pull secrets

* doc: add imagePullSecrets in kubernetes default example

(cherry picked from commit 053b320)

# Conflicts:
#	deploy/helm/elastic-agent/values.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.x Automated backport to the 8.x branch with mergify backport-8.16 Automated backport with mergify enhancement New feature or request skip-changelog Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants