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

Add Support for AWS Dex Basic-Authentication #357

Merged
merged 1 commit into from
Jun 23, 2020
Merged

Add Support for AWS Dex Basic-Authentication #357

merged 1 commit into from
Jun 23, 2020

Conversation

PatrickXYS
Copy link
Member

Which issue is resolved by this Pull Request:
Resolves #328

Description of your changes:
Integrated Dex as basic authentication option in AWS Kubeflow.

@kubeflow-bot
Copy link

This change is Reviewable

@PatrickXYS
Copy link
Member Author

/assign @Jeffwan

@@ -483,13 +421,19 @@ func (aws *Aws) Generate(resources kftypes.ResourceEnum) error {
return errors.WithStack(err)
}

// TODO: AWS doesn't enable BasicAuth yet.
if aws.kfDef.Spec.UseBasicAuth {
if pluginSpec.GetEnableBasicAuth() {
Copy link
Member

Choose a reason for hiding this comment

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

em.. I am thinking if we should think it's basic auth or equivalent as coginito and oidc.
Technically, I think they are same. However, it's kind of a simple auth without extra AWS services.

@PatrickXYS
Copy link
Member Author

Made two changes:

  1. Remove "enableBasicAuth"
  2. Encrypt password in Kfctl.

Tested locally and it works

@Jeffwan
Copy link
Member

Jeffwan commented Jun 23, 2020

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jeffwan

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:

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

@PatrickXYS
Copy link
Member Author

/test kubeflow-kfctl-presubmit

@Jeffwan
Copy link
Member

Jeffwan commented Jun 23, 2020

@Jeffwan
Copy link
Member

Jeffwan commented Jun 23, 2020

/test kubeflow-kfctl-presubmit

@PatrickXYS
Copy link
Member Author

@Jeffwan Should I rebase your change #359?

@Jeffwan
Copy link
Member

Jeffwan commented Jun 23, 2020

@PatrickXYS I don't this so. These are unrelated.

@PatrickXYS
Copy link
Member Author

/retest

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jun 23, 2020
@PatrickXYS
Copy link
Member Author

@Jeffwan Not sure why the tests keep failing, I tried to rebase changes and see if it succeed.

@Jeffwan
Copy link
Member

Jeffwan commented Jun 23, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit 074b580 into kubeflow:master Jun 23, 2020
Jeffwan added a commit to Jeffwan/kfctl that referenced this pull request Jun 29, 2020
Jeffwan added a commit to Jeffwan/kfctl that referenced this pull request Jun 29, 2020
k8s-ci-robot pushed a commit that referenced this pull request Jun 29, 2020
* Add region into IRSA role name (#359)

* Add Support for AWS Dex Basic-Authentication (#357)

* Pretty print log messages IAM policy(from #327) (#362)
vpavlin pushed a commit to vpavlin/kfctl that referenced this pull request Jul 10, 2020
vpavlin pushed a commit to vpavlin/kfctl that referenced this pull request Jul 20, 2020
vpavlin pushed a commit to vpavlin/kfctl that referenced this pull request Jul 22, 2020
vpavlin pushed a commit to vpavlin/kfctl that referenced this pull request Jul 22, 2020
crobby pushed a commit to crobby/kfctl that referenced this pull request Feb 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Orchestrate dex for kfctl_aws manifest
4 participants