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

Add support for bearer token auth in Prow Jenkins controller #4210

Merged
merged 1 commit into from
Sep 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions prow/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ all: build test

HOOK_VERSION ?= 0.155
SINKER_VERSION ?= 0.17
DECK_VERSION ?= 0.43
DECK_VERSION ?= 0.44
SPLICE_VERSION ?= 0.27
TOT_VERSION ?= 0.5
HOROLOGIUM_VERSION ?= 0.8
PLANK_VERSION ?= 0.42
JENKINS-OPERATOR_VERSION ?= 0.40
JENKINS-OPERATOR_VERSION ?= 0.41
TIDE_VERSION ?= 0.2

# These are the usual GKE variables.
Expand Down
8 changes: 7 additions & 1 deletion prow/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,15 @@ state and no claims of backwards compatibility are made for any external API.
```
kubectl label pods --all -n pod_namespace created-by-prow=true
```
- *September 1, 2017* `deck` version 0.44 and `jenkins-operator` version 0.41
controllers no longer provide a default value for the `--jenkins-token-file` flag.
Cluster administrators should provide `--jenkins-token-file=/etc/jenkins/jenkins`
explicitly when upgrading to a new version of these components if they were
previously relying on the default. For more context, please see
[this pull request.](https://github.com/kubernetes/test-infra/pull/4210)
- *August 29, 2017* Configuration specific to plugins is now held in in the
`plugins` `ConfigMap` and serialized in this repo in the `plugins.yaml` file.
Cluster administrators upgrading to the newest version of Prow should move
Cluster administrators upgrading to `hook` version 0.148 or newer should move
plugin configuration from the main `ConfigMap`. For more context, please see
[this pull request.](https://github.com/kubernetes/test-infra/pull/4213)

Expand Down
3 changes: 2 additions & 1 deletion prow/cluster/deck_deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ spec:
terminationGracePeriodSeconds: 30
containers:
- name: deck
image: gcr.io/k8s-prow/deck:0.43
image: gcr.io/k8s-prow/deck:0.44
ports:
- name: http
containerPort: 8080
args:
- --jenkins-url=$(JENKINS_URL)
- --jenkins-token-file=/etc/jenkins/jenkins
- --build-cluster=/etc/cluster/cluster
env:
- name: JENKINS_URL
Expand Down
3 changes: 2 additions & 1 deletion prow/cluster/jenkins_deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ spec:
spec:
containers:
- name: jenkins-operator
image: gcr.io/k8s-prow/jenkins-operator:0.39
image: gcr.io/k8s-prow/jenkins-operator:0.41
args:
- --dry-run=false
- --jenkins-token-file=/etc/jenkins/jenkins
volumeMounts:
- mountPath: /etc/jenkins
name: jenkins
Expand Down
41 changes: 33 additions & 8 deletions prow/cmd/deck/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ var (
configPath = flag.String("config-path", "/etc/config/config", "Path to config.yaml.")
buildCluster = flag.String("build-cluster", "", "Path to file containing a YAML-marshalled kube.Cluster object. If empty, uses the local cluster.")

jenkinsURL = flag.String("jenkins-url", "", "Jenkins URL")
jenkinsUserName = flag.String("jenkins-user", "jenkins-trigger", "Jenkins username")
jenkinsTokenFile = flag.String("jenkins-token-file", "/etc/jenkins/jenkins", "Path to the file containing the Jenkins API token.")
jenkinsURL = flag.String("jenkins-url", "", "Jenkins URL")
jenkinsUserName = flag.String("jenkins-user", "jenkins-trigger", "Jenkins username")
jenkinsTokenFile = flag.String("jenkins-token-file", "", "Path to the file containing the Jenkins API token.")
jenkinsBearerTokenFile = flag.String("jenkins-bearer-token-file", "", "Path to the file containing the Jenkins API bearer token.")
)

// Matches letters, numbers, hyphens, and underscores.
Expand Down Expand Up @@ -70,14 +71,30 @@ func main() {
}
}

var ac *jenkins.AuthConfig
var jc *jenkins.Client
if *jenkinsURL != "" {
jenkinsSecretRaw, err := ioutil.ReadFile(*jenkinsTokenFile)
if err != nil {
logrus.WithError(err).Fatalf("Could not read token file.")
if *jenkinsTokenFile != "" {
if token, err := loadToken(*jenkinsTokenFile); err != nil {
logrus.WithError(err).Fatalf("Could not read token file.")
} else {
ac.Basic = &jenkins.BasicAuthConfig{
User: *jenkinsUserName,
Token: token,
}
}
} else if *jenkinsBearerTokenFile != "" {
if token, err := loadToken(*jenkinsBearerTokenFile); err != nil {
logrus.WithError(err).Fatalf("Could not read token file.")
} else {
ac.BearerToken = &jenkins.BearerTokenAuthConfig{
Token: token,
}
}
} else {
logrus.Fatal("An auth token for basic or bearer token auth must be supplied.")
Copy link
Contributor

Choose a reason for hiding this comment

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

deck is used only for logs and on public jenkins instances we don't even need basic auth so we shouldn't break this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code prior to this change is

	if *jenkinsURL != "" {
		jenkinsSecretRaw, err := ioutil.ReadFile(*jenkinsTokenFile)
		if err != nil {
			logrus.WithError(err).Fatalf("Could not read token file.")
		}
		jenkinsToken := string(bytes.TrimSpace(jenkinsSecretRaw))
		jc = jenkins.NewClient(*jenkinsURL, *jenkinsUserName, jenkinsToken)
	}

How does that allow for no token?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh well.. I think I had a branch where I was making optional the parsing of the file because it's not really needed (tested). Nevermind, it's ok for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

It allows for no token if *jenkinsURL is empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

It won't work without a url.

Copy link
Contributor

Choose a reason for hiding this comment

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

It won't be able to look up any agent: jenkins jobs, but it will be able to do everything else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I was referring to getting the logs from Jenkins.

}
jenkinsToken := string(bytes.TrimSpace(jenkinsSecretRaw))
jc = jenkins.NewClient(*jenkinsURL, *jenkinsUserName, jenkinsToken)
jc = jenkins.NewClient(*jenkinsURL, ac)
}

ja := &JobAgent{
Expand All @@ -95,6 +112,14 @@ func main() {
logrus.WithError(http.ListenAndServe(":8080", nil)).Fatal("ListenAndServe returned.")
}

func loadToken(file string) (string, error) {
raw, err := ioutil.ReadFile(file)
if err != nil {
return "", err
}
return string(bytes.TrimSpace(raw)), nil
}

func handleData(ja *JobAgent) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Cache-Control", "no-cache")
Expand Down
41 changes: 33 additions & 8 deletions prow/cmd/jenkins-operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ import (
var (
configPath = flag.String("config-path", "/etc/config/config", "Path to config.yaml.")

jenkinsURL = flag.String("jenkins-url", "http://jenkins-proxy", "Jenkins URL")
jenkinsUserName = flag.String("jenkins-user", "jenkins-trigger", "Jenkins username")
jenkinsTokenFile = flag.String("jenkins-token-file", "/etc/jenkins/jenkins", "Path to the file containing the Jenkins API token.")
jenkinsURL = flag.String("jenkins-url", "http://jenkins-proxy", "Jenkins URL")
jenkinsUserName = flag.String("jenkins-user", "jenkins-trigger", "Jenkins username")
jenkinsTokenFile = flag.String("jenkins-token-file", "", "Path to the file containing the Jenkins API token.")
jenkinsBearerTokenFile = flag.String("jenkins-bearer-token-file", "", "Path to the file containing the Jenkins API bearer token.")

_ = flag.String("github-bot-name", "", "Deprecated.")
githubEndpoint = flag.String("github-endpoint", "https://api.github.com", "GitHub's API endpoint.")
Expand All @@ -58,12 +59,28 @@ func main() {
logrus.WithError(err).Fatal("Error getting kube client.")
}

jenkinsSecretRaw, err := ioutil.ReadFile(*jenkinsTokenFile)
if err != nil {
logrus.WithError(err).Fatalf("Could not read Jenkins token file.")
var ac *jenkins.AuthConfig
if *jenkinsTokenFile != "" {
token, err := loadToken(*jenkinsTokenFile)
if err != nil {
logrus.WithError(err).Fatalf("Could not read token file.")
}
ac.Basic = &jenkins.BasicAuthConfig{
User: *jenkinsUserName,
Token: token,
}
} else if *jenkinsBearerTokenFile != "" {
token, err := loadToken(*jenkinsBearerTokenFile)
if err != nil {
logrus.WithError(err).Fatalf("Could not read bearer token file.")
}
ac.BearerToken = &jenkins.BearerTokenAuthConfig{
Token: token,
}
} else {
logrus.Fatal("An auth token for basic or bearer token auth must be supplied.")
}
jenkinsToken := string(bytes.TrimSpace(jenkinsSecretRaw))
jc := jenkins.NewClient(*jenkinsURL, *jenkinsUserName, jenkinsToken)
jc := jenkins.NewClient(*jenkinsURL, ac)

oauthSecretRaw, err := ioutil.ReadFile(*githubTokenFile)
if err != nil {
Expand Down Expand Up @@ -95,3 +112,11 @@ func main() {
logrus.Infof("Sync time: %v", time.Since(start))
}
}

func loadToken(file string) (string, error) {
raw, err := ioutil.ReadFile(file)
if err != nil {
return "", err
}
return string(bytes.TrimSpace(raw)), nil
}
39 changes: 29 additions & 10 deletions prow/jenkins/jenkins.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,25 @@ type Status struct {
}

type Client struct {
client *http.Client
baseURL string
user string
token string
client *http.Client
baseURL string
authConfig *AuthConfig
}

// AuthConfig configures how we auth with Jenkins.
// Only one of the fields will be non-nil.
type AuthConfig struct {
Basic *BasicAuthConfig
BearerToken *BearerTokenAuthConfig
}

type BasicAuthConfig struct {
User string
Token string
}

type BearerTokenAuthConfig struct {
Token string
}

type BuildRequest struct {
Expand All @@ -60,12 +75,11 @@ type Build struct {
QueueURL *url.URL
}

func NewClient(url, user, token string) *Client {
func NewClient(url string, authConfig *AuthConfig) *Client {
return &Client{
baseURL: url,
user: user,
token: token,
client: &http.Client{},
baseURL: url,
authConfig: authConfig,
client: &http.Client{},
}
}

Expand Down Expand Up @@ -93,7 +107,12 @@ func (c *Client) doRequest(method, path string) (*http.Response, error) {
if err != nil {
return nil, err
}
req.SetBasicAuth(c.user, c.token)
if c.authConfig.Basic != nil {
req.SetBasicAuth(c.authConfig.Basic.User, c.authConfig.Basic.Token)
}
if c.authConfig.BearerToken != nil {
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.authConfig.BearerToken.Token))
}
return c.client.Do(req)
}

Expand Down