Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Add option to use project name as namespace for the task pods #166

Merged
merged 2 commits into from
Mar 25, 2021
Merged

Add option to use project name as namespace for the task pods #166

merged 2 commits into from
Mar 25, 2021

Conversation

jeevb
Copy link
Contributor

@jeevb jeevb commented Mar 24, 2021 •
edited
Loading

TL;DR

Use project name as namespace for task pods

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

https://github.com/lyft/flyte/issues/

Follow-up issue

NA
OR
https://github.com/lyft/flyte/issues/

@jeevb jeevb force-pushed the jeev/project-namespace branch from e2bf7d1 to 37eded8 Compare March 24, 2021 18:47
Add option to use project name as namespace for the task pods
ca7607d 
Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
@jeevb jeevb force-pushed the jeev/project-namespace branch from 37eded8 to ca7607d Compare March 24, 2021 18:54
jeevb
jeevb commented Mar 24, 2021
View reviewed changes
pkg/common/namespace.go Outdated
Domain NamespaceMapping = iota
NmProjectDomain NamespaceMapping = iota
NmDomain NamespaceMapping = iota
NmProject NamespaceMapping = iota
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Project is already declared in the package, so I just prefixed all of these with Nm. Let me know if there is a preferred practice around this.

@codecov
Copy link

codecov bot commented Mar 24, 2021 •
edited
Loading

Codecov Report

Merging #166 (02ad29e) into master (24d3ece) will decrease coverage by 0.00%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
- Coverage   62.79%   62.78%   -0.01%     
==========================================
  Files         100      100              
  Lines        7284     7288       +4     
==========================================
+ Hits         4574     4576       +2     
- Misses       2124     2126       +2     
  Partials      586      586
Flag Coverage Δ
unittests 62.78% <44.44%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
pkg/runtime/namespace_config_provider.go 7.14% <0.00%> (-1.20%) ⬇️
pkg/common/namespace.go 100.00% <100.00%> (ø)

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 24d3ece...02ad29e. Read the comment docs.

@jeevb jeevb requested review from migueltol22 and katrogan March 24, 2021 19:11
@jeevb jeevb marked this pull request as ready for review March 24, 2021 19:12
EngHabu
EngHabu reviewed Mar 24, 2021
View reviewed changes
pkg/common/namespace.go Outdated
Comment on lines 10 to 12
NmProjectDomain NamespaceMapping = iota
NmDomain NamespaceMapping = iota
NmProject NamespaceMapping = iota
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
NmProjectDomain NamespaceMapping = iota
NmDomain NamespaceMapping = iota
NmProject NamespaceMapping = iota
NamespaceMappingProjectDomain NamespaceMapping = iota
NamespaceMappingDomain
NamespaceMappingProject

rename
02ad29e 
Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
@jeevb jeevb requested a review from EngHabu March 24, 2021 19:34
EngHabu
EngHabu approved these changes Mar 25, 2021
View reviewed changes
Copy link
Contributor

@EngHabu EngHabu 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 to me, I would like to get @katrogan to take a look though...

@EngHabu EngHabu merged commit 1ef63b1 into flyteorg:master Mar 25, 2021
@jeevb jeevb deleted the jeev/project-namespace branch March 25, 2021 21:13
EngHabu pushed a commit that referenced this pull request Apr 26, 2021
@jeevb @EngHabu
Add option to use project name as namespace for the task pods (#166)
64dd4fe 
* Add option to use project name as namespace for the task pods

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>

* rename

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
EngHabu pushed a commit that referenced this pull request Apr 26, 2021
@jeevb @EngHabu
Add option to use project name as namespace for the task pods (#166)
912d2e2 
* Add option to use project name as namespace for the task pods

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>

* rename

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
EngHabu added a commit that referenced this pull request Apr 30, 2021
@EngHabu @pmahindrakar-oss
@anandswaminathan @jeevb @yindia @flyte-bot
[Breaking] OAuth2 Authorization Server implementation, Separate OpenI…
611a85f 
…D and OAuth2 configs, OAuth2 Metadata over gRPC #minor (#168)

* wip: OAuth2 Support

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* tighten security of generated tokens

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support storing form post values in auth code JWT

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* save secrets to k8s secrets

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Expose metadata endpoints over gRPC

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* trim OpenID Connect config further

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Selectively authenticate gRPC endpoints

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support external oauth2 server and Okta Config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* update config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix nil secrets data map

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fixed the pointer overwrite issue in oauthServer metadata (#183)

Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>

Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Simplify config further and move auth package up

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix clusterresource Project and domain(#167)

* Fix clusterresource Project

Signed-off-by: Anand Swaminathan <aswaminathan@lyft.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Bump flyteidl version to pick up auth role field number fix (#169)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add option to use project name as namespace for the task pods (#166)

* Add option to use project name as namespace for the task pods

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>

* rename

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* GetExecution performance improvements (#171)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add exists check for workflow & node executions (#172)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Remove legacy fetch for workflow execution inputs (#173)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added release workflow (#170)

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update Flyteidl version (#175)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added version in flyteadmin (#154)

* wip: added version pkg

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: resolve conflict

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added version in rpc

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: small fixes

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: Added panic cache in get version service

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added flytestdlib for version package

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added version service test

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added ldflags in goreleaser

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Propagate nesting and principal for child executions (#177)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Write workflow and node execution events asynchronously (#174)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add sensible flyteadmin config defaults (#179)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Lint

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* further cleanup

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Only register authserver when auth is enabled

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to latest flyteidl and separate auth interfaces

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* dead code

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* PR Comments

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* merge master

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Move to authorizedUris

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to released flyteidl

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix response expiry and add unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update go mod

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* fix unit tests that broke because of identity changes

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Co-authored-by: pmahindrakar-oss <77798312+pmahindrakar-oss@users.noreply.github.com>
Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Co-authored-by: Anand Swaminathan <aswaminathan@lyft.com>
Co-authored-by: Katrina Rogan <katrina@nuclyde.io>
Co-authored-by: Jeev B <jeevb@users.noreply.github.com>
Co-authored-by: Yuvraj <10830562+evalsocket@users.noreply.github.com>
Co-authored-by: Flyte Bot <admin@flyte.org>
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
@jeevb
Add option to use project name as namespace for the task pods (#166)
4fc817c 
* Add option to use project name as namespace for the task pods

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>

* rename

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
@EngHabu @pmahindrakar-oss
@anandswaminathan @jeevb @yindia @flyte-bot
[Breaking] OAuth2 Authorization Server implementation, Separate OpenI…
435f520 
…D and OAuth2 configs, OAuth2 Metadata over gRPC #minor (#168)

* wip: OAuth2 Support

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* tighten security of generated tokens

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support storing form post values in auth code JWT

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* save secrets to k8s secrets

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Expose metadata endpoints over gRPC

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* trim OpenID Connect config further

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Selectively authenticate gRPC endpoints

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support external oauth2 server and Okta Config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* update config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix nil secrets data map

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fixed the pointer overwrite issue in oauthServer metadata (#183)

Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>

Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Simplify config further and move auth package up

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix clusterresource Project and domain(#167)

* Fix clusterresource Project

Signed-off-by: Anand Swaminathan <aswaminathan@lyft.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Bump flyteidl version to pick up auth role field number fix (#169)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add option to use project name as namespace for the task pods (#166)

* Add option to use project name as namespace for the task pods

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>

* rename

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* GetExecution performance improvements (#171)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add exists check for workflow & node executions (#172)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Remove legacy fetch for workflow execution inputs (#173)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added release workflow (#170)

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update Flyteidl version (#175)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added version in flyteadmin (#154)

* wip: added version pkg

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: resolve conflict

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added version in rpc

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: small fixes

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: Added panic cache in get version service

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added flytestdlib for version package

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added version service test

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added ldflags in goreleaser

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Propagate nesting and principal for child executions (#177)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Write workflow and node execution events asynchronously (#174)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add sensible flyteadmin config defaults (#179)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Lint

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* further cleanup

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Only register authserver when auth is enabled

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to latest flyteidl and separate auth interfaces

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* dead code

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* PR Comments

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* merge master

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Move to authorizedUris

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to released flyteidl

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix response expiry and add unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update go mod

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* fix unit tests that broke because of identity changes

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Co-authored-by: pmahindrakar-oss <77798312+pmahindrakar-oss@users.noreply.github.com>
Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Co-authored-by: Anand Swaminathan <aswaminathan@lyft.com>
Co-authored-by: Katrina Rogan <katrina@nuclyde.io>
Co-authored-by: Jeev B <jeevb@users.noreply.github.com>
Co-authored-by: Yuvraj <10830562+evalsocket@users.noreply.github.com>
Co-authored-by: Flyte Bot <admin@flyte.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@katrogan katrogan katrogan approved these changes

@EngHabu EngHabu EngHabu approved these changes

@migueltol22 migueltol22 Awaiting requested review from migueltol22

@anandswaminathan anandswaminathan Awaiting requested review from anandswaminathan

@kumare3 kumare3 Awaiting requested review from kumare3 kumare3 is a code owner

@wild-endeavor wild-endeavor Awaiting requested review from wild-endeavor wild-endeavor is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jeevb @EngHabu @katrogan