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: Add support for jamf_service to the teleport-kube-agent Helm chart #42368

Merged
merged 22 commits into from
Jun 7, 2024

Conversation

lcharkiewicz
Copy link
Contributor

@lcharkiewicz lcharkiewicz commented Jun 4, 2024

This PR adds support for jamf_service configuration to the teleport-kube-agent Helm chart.

The change supports Jamf API Client Credentials auth method available in Teleport v16+

More about the required configuration can be found in the official Teleport docs.

Additional Jamf service options, like sync_period_* or filter_rsql can be configured by setting teleportConfig.

changelog: helm: add support for jamf_service to teleport-kube-agent

@lcharkiewicz lcharkiewicz requested review from hugoShaka and tigrato June 4, 2024 14:46
Copy link
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

For v16+ we should be pushing folks toward using client ID and client secret instead of username and password. cc @codingllama

Copy link

github-actions bot commented Jun 4, 2024

🤖 Vercel preview here: https://docs-m06cobads-goteleport.vercel.app/docs/ver/preview

@codingllama
Copy link
Contributor

For v16+ we should be pushing folks toward using client ID and client secret instead of username and password. cc @codingllama

+1, please add fields for client ID and client secret, and document that these are preferred over username/password. All that is required already landed for v16 (see #41928 or public docs for reference).

@codingllama
Copy link
Contributor

@hugoShaka
Copy link
Contributor

If we're going to ship this PR in v16, we can even remove the user/password fields so folks have to use client ID/secret.

Copy link

github-actions bot commented Jun 4, 2024

🤖 Vercel preview here: https://docs-3y9wt21dm-goteleport.vercel.app/docs/ver/preview

@lcharkiewicz
Copy link
Contributor Author

If we're going to ship this PR in v16, we can even remove the user/password fields so folks have to use client ID/secret.

Currently both auth methods are supported. If keeping only Client Credentials is preferred, I'm happy to remove it.

@lcharkiewicz lcharkiewicz marked this pull request as ready for review June 5, 2024 12:30
Copy link

github-actions bot commented Jun 5, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@github-actions github-actions bot requested a review from marcoandredinis June 5, 2024 12:31
examples/chart/teleport-kube-agent/templates/_config.tpl Outdated Show resolved Hide resolved
examples/chart/teleport-kube-agent/templates/secret.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-kube-agent/tests/secret_test.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-kube-agent/values.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-kube-agent/values.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-kube-agent/values.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-kube-agent/templates/_config.tpl Outdated Show resolved Hide resolved
@rosstimothy
Copy link
Contributor

If we're going to ship this PR in v16, we can even remove the user/password fields so folks have to use client ID/secret.

Currently both auth methods are supported. If keeping only Client Credentials is preferred, I'm happy to remove it.

@lcharkiewicz do we need this chart for any versions prior to v16? If no, then let's eliminate the user/password entirely and just ship the client credentials. That would simplify the chart a bit since the jamf client credentials are only supported on v16.

Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
Copy link

github-actions bot commented Jun 6, 2024

🤖 Vercel preview here: https://docs-ioxcmqfsp-goteleport.vercel.app/docs/ver/preview

Copy link

github-actions bot commented Jun 6, 2024

🤖 Vercel preview here: https://docs-g1hd4tdsg-goteleport.vercel.app/docs/ver/preview

@lcharkiewicz
Copy link
Contributor Author

If we're going to ship this PR in v16, we can even remove the user/password fields so folks have to use client ID/secret.

Currently both auth methods are supported. If keeping only Client Credentials is preferred, I'm happy to remove it.

@lcharkiewicz do we need this chart for any versions prior to v16? If no, then let's eliminate the user/password entirely and just ship the client credentials. That would simplify the chart a bit since the jamf client credentials are only supported on v16.

@rosstimothy I thought my team might need it with v15. After a discussion we decided that we can go with v16 only. Thus, I dropped the support for Jamf username and password auth (and yes, I like the simpler code now :)).

@lcharkiewicz
Copy link
Contributor Author

@hugoShaka your suggestions have been addressed, PTAL.

@lcharkiewicz lcharkiewicz requested a review from hugoShaka June 6, 2024 20:25
@lcharkiewicz lcharkiewicz added this pull request to the merge queue Jun 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 7, 2024
Copy link

github-actions bot commented Jun 7, 2024

🤖 Vercel preview here: https://docs-730x5znld-goteleport.vercel.app/docs/ver/preview

@lcharkiewicz lcharkiewicz added this pull request to the merge queue Jun 7, 2024
Merged via the queue into master with commit 1c14034 Jun 7, 2024
38 checks passed
@lcharkiewicz lcharkiewicz deleted the lcharkiewicz/helm-chart-jamf-service-support branch June 7, 2024 07:32
@public-teleport-github-review-bot

@lcharkiewicz See the table below for backport results.

Branch Result
branch/v16 Create PR

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.

5 participants