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

Remove --credentials flags, stop generating router/registry client certs #10830

Merged
merged 2 commits into from
Jan 25, 2017
Merged

Remove --credentials flags, stop generating router/registry client certs #10830

merged 2 commits into from
Jan 25, 2017

Conversation

liggitt
Copy link
Contributor

@liggitt liggitt commented Sep 7, 2016

Fixes #3951

doc in openshift/openshift-docs#3552

ansible hasn't used this since 3.2

@liggitt liggitt added this to the 1.4.0 milestone Sep 7, 2016
@liggitt
Copy link
Contributor Author

liggitt commented Sep 7, 2016

[test]

@liggitt liggitt removed this from the 1.4.0 milestone Sep 15, 2016
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2016
@ghost
Copy link

ghost commented Sep 22, 2016

Do we have an ETA on when this will be cherry picked into OpenShift Enterprise 3.x? Then I'll attach CVE-2015-8945 to the errata in question and we can close this out.

@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 24, 2016
@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 4, 2016
@liggitt liggitt changed the title Make deprecated --credentials flags inert Remove --credentials flags Jan 20, 2017
@liggitt
Copy link
Contributor Author

liggitt commented Jan 20, 2017

[test]

@liggitt liggitt changed the title Remove --credentials flags Remove --credentials flags, stop generating router/registry client certs Jan 20, 2017
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2017
@liggitt
Copy link
Contributor Author

liggitt commented Jan 20, 2017

@deads2k @sdodson PTAL

@sdodson
Copy link
Member

sdodson commented Jan 20, 2017

I don't see any problems with this and our current installer code.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 22, 2017
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2017
}

needServiceAccountRole := len(opts.Config.ServiceAccount) > 0 && len(opts.Config.Credentials) == 0
needServiceAccountRole := len(opts.Config.ServiceAccount) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't you just make sure they specified it? this bool seems unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

}
createServiceAccount := len(cfg.ServiceAccount) > 0 && len(cfg.Credentials) == 0
createServiceAccount := len(cfg.ServiceAccount) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

always true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, removed

@deads2k
Copy link
Contributor

deads2k commented Jan 24, 2017

minor comments. lgtm otherwise.

@liggitt
Copy link
Contributor Author

liggitt commented Jan 24, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 9a86874

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13275/) (Base Commit: ed6d1ad)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 9a86874

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 25, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13292/) (Base Commit: a32dda3) (Image: devenv-rhel7_5774)

@openshift-bot openshift-bot merged commit f762dc6 into openshift:master Jan 25, 2017
@liggitt liggitt deleted the remove-credentials branch February 20, 2017 13:43
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Mar 18, 2017
This is primarily to get the signature access docker/distribution API
extension.

To make it work, two updates to the test harness are necessary:

- Change the expected output of (oadm policy add-cluster-role-to-group)
- Don't expect (openshift start master) to create .kubeconfig files
  for the registry service.

  As of openshift/origin#10830 ,
  openshift.local.config/master/openshift-registry.kubeconfig is no longer
  autogenerated.  Instead, do what (oadm registry) does, creating a
  service account and a cluster policy role binding.  Then manually create
  the necessary certificates and a .kubeconfig instead of using the
  service account in a pod.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants