Add support for `serviceAccountName` on deploy #1811

saschagrunert · 2023-06-12T08:01:29Z

Changes

This allows setting the service account for the resulting knative service, which must pre-exist in the namespace to let the deployment succeed.

/kind feature

Release Note

Added support for `serviceAccountName` in `func.yaml`'s deploy section to set the function service account.

Docs

None

knative-prow · 2023-06-12T08:01:31Z

@saschagrunert: The label(s) kind/feature cannot be applied, because the repository doesn't have them.

In response to this:

Changes

This allows setting the service account for the resulting knative service, which must pre-exist in the namespace to let the deployment succeed.

/kind feature

Release Note
Added support for `serviceAccountName` in `func.yaml`'s deploy section to set the function service account.
Docs
None

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.

saschagrunert · 2023-06-12T08:03:00Z

The intention of this PR is to discuss if such a feature is valuable to the community. I have a use case for it and would like to restrict the capabilities of the service without using the default service account (or modifying the service afterwards).

/hold

codecov · 2023-06-12T08:17:01Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.78 🎉

Comparison is base (529957e) 62.50% compared to head (d25741b) 63.28%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1811      +/-   ##
==========================================
+ Coverage   62.50%   63.28%   +0.78%     
==========================================
  Files         106      105       -1     
  Lines       13422    13421       -1     
==========================================
+ Hits         8389     8494     +105     
+ Misses       4246     4114     -132     
- Partials      787      813      +26

Flag	Coverage Δ
e2e-test	`37.60% <100.00%> (-0.09%)`	⬇️
e2e-test-oncluster	`33.40% <100.00%> (+0.09%)`	⬆️
e2e-test-oncluster-runtime	`28.58% <0.00%> (?)`
e2e-test-runtime-go	`27.33% <100.00%> (?)`
e2e-test-runtime-node	`28.40% <100.00%> (?)`
e2e-test-runtime-python	`28.40% <100.00%> (?)`
e2e-test-runtime-quarkus	`28.52% <100.00%> (?)`
e2e-test-runtime-springboot	`27.48% <100.00%> (?)`
e2e-test-runtime-typescript	`28.52% <100.00%> (?)`
integration-tests	`52.07% <100.00%> (+3.67%)`	⬆️
unit-tests-macos-latest	`?`
unit-tests-ubuntu-latest	`50.75% <0.00%> (+0.07%)`	⬆️
unit-tests-windows-latest	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/functions/function.go	`85.80% <ø> (ø)`
pkg/knative/deployer.go	`64.25% <100.00%> (+0.05%)`	⬆️

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

zroubalik

Nice, could you please also update docs? https://github.com/knative/func/blob/main/docs/reference/func_yaml.md

This allows setting the service account for the resulting knative service, which must pre-exist in the namespace to let the deployment succeed. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>

saschagrunert · 2023-06-12T12:42:29Z

Nice, could you please also update docs? https://github.com/knative/func/blob/main/docs/reference/func_yaml.md

Yep done ✔️

lance

/lgtm

knative-prow · 2023-06-13T20:13:01Z

@saschagrunert: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
integration-tests-latest-release_func_main	`d25741b`	link	true	`/test integration-tests-latest-release`

Your PR dashboard.

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. I understand the commands that are listed here.

lance · 2023-06-13T20:14:21Z

@saschagrunert yes, I think this is a useful feature. It looks like someone else also wants it! #1812

I think we should merge this PR and then @nitishchauhan0022 can rebase their PR which adds a flag for it to the func deploy command.

saschagrunert · 2023-06-14T07:16:28Z

/test integration-tests-latest-release

knative-prow · 2023-06-14T07:16:43Z

@saschagrunert: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test integration-tests-latest-release

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.

matejvasek · 2023-06-19T14:37:07Z

/hold

matejvasek · 2023-06-19T14:46:01Z

@saschagrunert isn't #1812 superset of this PR?

matejvasek · 2023-06-19T16:46:05Z

/approve
/lgtm
/unhold

knative-prow · 2023-06-19T16:46:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matejvasek, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [matejvasek]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

saschagrunert · 2023-06-20T07:01:59Z

@saschagrunert isn't #1812 superset of this PR?

Yes, probably. :)

lance · 2023-07-03T14:15:52Z

/cherry-pick release-1.10

knative-prow-robot · 2023-07-03T14:16:40Z

@lance: new pull request created: #1842

In response to this:

/cherry-pick release-1.10

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.

knative-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 12, 2023

knative-prow bot requested review from jrangelramos and nainaz June 12, 2023 08:01

knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 12, 2023

zroubalik reviewed Jun 12, 2023

View reviewed changes

Add support for serviceAccountName on deploy

d25741b

This allows setting the service account for the resulting knative service, which must pre-exist in the namespace to let the deployment succeed. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>

saschagrunert force-pushed the service-account branch from bd360eb to d25741b Compare June 12, 2023 12:42

saschagrunert changed the title ~~RFC: Add support for serviceAccountName on deploy~~ Add support for serviceAccountName on deploy Jun 12, 2023

lance reviewed Jun 13, 2023

View reviewed changes

knative-prow bot assigned lance Jun 13, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2023

knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 19, 2023

knative-prow bot assigned matejvasek Jun 19, 2023

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 19, 2023

knative-prow bot merged commit c1a90f6 into knative:main Jun 19, 2023
38 checks passed

saschagrunert deleted the service-account branch June 20, 2023 07:01

knative-prow-robot mentioned this pull request Jul 3, 2023

[release-1.10] Add support for serviceAccountName on deploy #1842

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for `serviceAccountName` on deploy #1811

Add support for `serviceAccountName` on deploy #1811

saschagrunert commented Jun 12, 2023 •

edited

Loading

knative-prow bot commented Jun 12, 2023

Changes

saschagrunert commented Jun 12, 2023 •

edited

Loading

codecov bot commented Jun 12, 2023 •

edited

Loading

zroubalik left a comment

saschagrunert commented Jun 12, 2023 •

edited

Loading

lance left a comment

knative-prow bot commented Jun 13, 2023

lance commented Jun 13, 2023

saschagrunert commented Jun 14, 2023

knative-prow bot commented Jun 14, 2023

matejvasek commented Jun 19, 2023

matejvasek commented Jun 19, 2023

matejvasek commented Jun 19, 2023

knative-prow bot commented Jun 19, 2023

saschagrunert commented Jun 20, 2023

lance commented Jul 3, 2023

knative-prow-robot commented Jul 3, 2023

Add support for serviceAccountName on deploy #1811

Add support for serviceAccountName on deploy #1811

Conversation

saschagrunert commented Jun 12, 2023 • edited Loading

Changes

knative-prow bot commented Jun 12, 2023

Changes

saschagrunert commented Jun 12, 2023 • edited Loading

codecov bot commented Jun 12, 2023 • edited Loading

Codecov Report

zroubalik left a comment

Choose a reason for hiding this comment

saschagrunert commented Jun 12, 2023 • edited Loading

lance left a comment

Choose a reason for hiding this comment

knative-prow bot commented Jun 13, 2023

lance commented Jun 13, 2023

saschagrunert commented Jun 14, 2023

knative-prow bot commented Jun 14, 2023

matejvasek commented Jun 19, 2023

matejvasek commented Jun 19, 2023

matejvasek commented Jun 19, 2023

knative-prow bot commented Jun 19, 2023

saschagrunert commented Jun 20, 2023

lance commented Jul 3, 2023

knative-prow-robot commented Jul 3, 2023

Add support for `serviceAccountName` on deploy #1811

Add support for `serviceAccountName` on deploy #1811

saschagrunert commented Jun 12, 2023 •

edited

Loading

saschagrunert commented Jun 12, 2023 •

edited

Loading

codecov bot commented Jun 12, 2023 •

edited

Loading

saschagrunert commented Jun 12, 2023 •

edited

Loading