[FEATURE] - Run Jobs in central namespace #295

gambol99 · 2022-04-19T07:24:09Z

Currently all jobs are run in the namespace the configuration CRD is created in, which given how the provider secret implementation works (copying the secret into the desiganated namespace) exposes central credentials. This PR adds a conntroller flag (--controller-namespace) which runs all jobs in a specific namespace and ensuring any secrets are never exposed beyond that boundary.

Also add the ability to run the jobs under a serviceaccount, extending the Provider CRD. When run via the --job-namespace the platform administrator can provision a service account with Pod identity (EKS IRSA) and remove the need for static secrets.

msftgits · 2022-04-19T07:24:21Z

All CLA requirements met.

controllers/provider_controller.go

codecov · 2022-04-19T14:48:48Z

Codecov Report

Merging #295 (8bee771) into master (3b4fde8) will decrease coverage by 1.07%.
The diff coverage is 80.19%.

@@            Coverage Diff             @@
##           master     #295      +/-   ##
==========================================
- Coverage   81.84%   80.76%   -1.08%     
==========================================
  Files          19       19              
  Lines        1333     1383      +50     
==========================================
+ Hits         1091     1117      +26     
- Misses        174      196      +22     
- Partials       68       70       +2

Flag	Coverage Δ
unit	`80.76% <80.19%> (-1.08%)`	⬇️

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

Impacted Files	Coverage Δ
controllers/configuration_controller.go	`75.42% <70.14%> (-1.81%)`	⬇️
controllers/provider/credentials.go	`92.85% <100.00%> (+0.38%)`	⬆️
controllers/provider_controller.go	`89.74% <100.00%> (+2.64%)`	⬆️
controllers/terraform/logging.go	`66.00% <100.00%> (+0.69%)`	⬆️
controllers/terraform/status.go	`77.41% <100.00%> (ø)`
controllers/rbac.go	`89.33% <0.00%> (-1.34%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b4fde8...8bee771. Read the comment docs.

chart/values.yaml

controllers/configuration_controller.go

zzxwill · 2022-04-26T07:09:13Z

controllers/provider_controller.go

 		return ctrl.Result{}, errors.Wrap(updateErr, errSettingStatus)
 	}

-	return ctrl.Result{}, nil
+	return ctrl.Result{}, err


Is the err https://github.com/oam-dev/terraform-controller/pull/295/files#diff-8d523b8a346b8e0143bd91409158007d7870cee18c5facd66f70b2c94c8c2f1fR65?

Indeed .. though admitted not the best way of doing it .. i wasn't sure where else as you always want the Status() to be updated.

Then we had better not return err here. The rest are LGTM.

main.go

zzxwill

Great job!
Here are two minor questions, please take a look.

wonderflow

good job

zzxwill · 2022-07-16T03:39:29Z

@chivalryq I think we can have this PR merged to address the request from https://cloud-native.slack.com/archives/C01BLQ3HTJA/p1657554463584499?thread_ts=1656829163.511879&channel=C01BLQ3HTJA&message_ts=1657554463.584499.

chivalryq · 2022-07-16T03:54:00Z

@chivalryq I think we can have this PR merged to address the request from cloud-native.slack.com/archives/C01BLQ3HTJA/p1657554463584499?thread_ts=1656829163.511879&channel=C01BLQ3HTJA&message_ts=1657554463.584499.

Thanks for reminding! I'll take another review. BTW, could you resolve the conflict before we merge this? @gambol99

gambol99 · 2022-07-20T13:17:10Z

@chivalryq I think we can have this PR merged to address the request from cloud-native.slack.com/archives/C01BLQ3HTJA/p1657554463584499?thread_ts=1656829163.511879&channel=C01BLQ3HTJA&message_ts=1657554463.584499.

Thanks for reminding! I'll take another review. BTW, could you resolve the conflict before we merge this? @gambol99

Apologizes for the delay .. I'll fix this up now ..

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

…a central namespace and avoiding the need to copy global secrets around Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

…ocalizing to a single namespace if required Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- changing the variable name in controller and meta to reflect changes i.e JobNamespace -> ControllerNamespace - updated the helm chart to reflect the changes Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

anoop2811 · 2022-07-22T14:10:30Z

Hi Guys, justy following up on if this will be merged in time for 1.5.0 release? cc: @wonderflow @zzxwill

chivalryq · 2022-07-23T03:45:29Z

@anoop2811 Addon can be release apart from the KubeVela. So once @gambol99 finishes fixing and this is merged, we can release a patch version.

codecov · 2022-07-24T15:37:06Z

Codecov Report

❗ No coverage uploaded for pull request base (master@88aa8b9). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #295   +/-   ##
=========================================
  Coverage          ?   75.13%           
=========================================
  Files             ?       24           
  Lines             ?     1701           
  Branches          ?        0           
=========================================
  Hits              ?     1278           
  Misses            ?      344           
  Partials          ?       79

Flag	Coverage Δ
e2e	`0.00% <0.00%> (?)`
unit	`78.45% <0.00%> (?)`

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

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88aa8b9...eaeaa17. Read the comment docs.

…ng it works Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

gambol99 · 2022-07-24T16:14:06Z

Rebased and fixed up 👍🏼

wonderflow

Good job

zzxwill reviewed Apr 19, 2022

View reviewed changes

controllers/provider_controller.go Show resolved Hide resolved

gambol99 force-pushed the feature/job_namespace branch 5 times, most recently from 24a7fe2 to cf28b06 Compare April 19, 2022 14:45

gambol99 force-pushed the feature/job_namespace branch from 68e3994 to ea74383 Compare April 20, 2022 08:17

zzxwill reviewed Apr 20, 2022

View reviewed changes

chart/values.yaml Outdated Show resolved Hide resolved

zzxwill reviewed Apr 20, 2022

View reviewed changes

controllers/configuration_controller.go Outdated Show resolved Hide resolved

zzxwill reviewed Apr 20, 2022

View reviewed changes

controllers/configuration_controller.go Show resolved Hide resolved

gambol99 force-pushed the feature/job_namespace branch 4 times, most recently from 75e3482 to 8849414 Compare April 21, 2022 13:17

zzxwill reviewed Apr 26, 2022

View reviewed changes

main.go Outdated Show resolved Hide resolved

zzxwill approved these changes Apr 26, 2022

View reviewed changes

gambol99 force-pushed the feature/job_namespace branch from a8587aa to 8bee771 Compare May 3, 2022 08:46

wonderflow approved these changes May 9, 2022

View reviewed changes

gambol99 added 6 commits July 20, 2022 14:19

- updating the chart to support job namespaces

9963e85

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- updating the controllers to support the ability to run the jobs in …

a51525e

…a central namespace and avoiding the need to copy global secrets around Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- updating the cmd to support passing the job namespace and support l…

9673d1f

…ocalizing to a single namespace if required Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

adding the resource limits

42254b0

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- trying to fix the linting and unit tests

4cedcca

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- improving the test coverage

23f8b70

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

gambol99 added 4 commits July 20, 2022 14:45

- changing the job-namespace command line flag to --controller-namespace

f63fd1a

- changing the variable name in controller and meta to reflect changes i.e JobNamespace -> ControllerNamespace - updated the helm chart to reflect the changes Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- uncommenting the tests

6a24a8b

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- commenting out the test for a moment

5d24ee3

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

- fixing up the issue highlighted in the review

18671c9

Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

gambol99 force-pushed the feature/job_namespace branch from 8bee771 to ba3f22b Compare July 24, 2022 15:28

gambol99 force-pushed the feature/job_namespace branch from ba3f22b to c2ffc8e Compare July 24, 2022 15:49

- rebasing master onto the branch and fixing the units tests / ensuri…

eaeaa17

…ng it works Signed-off-by: Rohith Jayawardene <gambol99@gmail.com>

gambol99 force-pushed the feature/job_namespace branch from c2ffc8e to eaeaa17 Compare July 24, 2022 16:00

wonderflow approved these changes Jul 25, 2022

View reviewed changes

wonderflow merged commit 4b5a5eb into kubevela:master Jul 25, 2022

chivalryq mentioned this pull request Aug 8, 2022

Configuration won't get deleted when its namespace is being deleted. #329

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE] - Run Jobs in central namespace #295

[FEATURE] - Run Jobs in central namespace #295

gambol99 commented Apr 19, 2022 •

edited by chivalryq

Loading

msftgits commented Apr 19, 2022 •

edited

Loading

codecov bot commented Apr 19, 2022 •

edited

Loading

zzxwill Apr 26, 2022

gambol99 May 3, 2022

zzxwill May 5, 2022

zzxwill left a comment •

edited

Loading

wonderflow left a comment

zzxwill commented Jul 16, 2022

chivalryq commented Jul 16, 2022

gambol99 commented Jul 20, 2022

anoop2811 commented Jul 22, 2022

chivalryq commented Jul 23, 2022

codecov bot commented Jul 24, 2022 •

edited

Loading

gambol99 commented Jul 24, 2022

wonderflow left a comment

[FEATURE] - Run Jobs in central namespace #295

[FEATURE] - Run Jobs in central namespace #295

Conversation

gambol99 commented Apr 19, 2022 • edited by chivalryq Loading

msftgits commented Apr 19, 2022 • edited Loading

codecov bot commented Apr 19, 2022 • edited Loading

Codecov Report

zzxwill Apr 26, 2022

Choose a reason for hiding this comment

gambol99 May 3, 2022

Choose a reason for hiding this comment

zzxwill May 5, 2022

Choose a reason for hiding this comment

zzxwill left a comment • edited Loading

Choose a reason for hiding this comment

wonderflow left a comment

Choose a reason for hiding this comment

zzxwill commented Jul 16, 2022

chivalryq commented Jul 16, 2022

gambol99 commented Jul 20, 2022

anoop2811 commented Jul 22, 2022

chivalryq commented Jul 23, 2022

codecov bot commented Jul 24, 2022 • edited Loading

Codecov Report

gambol99 commented Jul 24, 2022

wonderflow left a comment

Choose a reason for hiding this comment

gambol99 commented Apr 19, 2022 •

edited by chivalryq

Loading

msftgits commented Apr 19, 2022 •

edited

Loading

codecov bot commented Apr 19, 2022 •

edited

Loading

zzxwill left a comment •

edited

Loading

codecov bot commented Jul 24, 2022 •

edited

Loading