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

Gitlab integration #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions core/src/main/resources/db/migration/V2_1__alter_release_id.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--: ----------------------------------------------------------------------------
--: Copyright (C) 2017 Verizon. All Rights Reserved.
--:
--: Licensed under the Apache License, Version 2.0 (the "License");
--: you may not use this file except in compliance with the License.
--: You may obtain a copy of the License at
--:
--: http://www.apache.org/licenses/LICENSE-2.0
--:
--: Unless required by applicable law or agreed to in writing, software
--: distributed under the License is distributed on an "AS IS" BASIS,
--: WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
--: See the License for the specific language governing permissions and
--: limitations under the License.
--:
--: ----------------------------------------------------------------------------
ALTER TABLE PUBLIC."audit_log" ALTER COLUMN "release_id" VARCHAR(25);
Copy link
Member

Choose a reason for hiding this comment

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

I am not familiar with DB migrations, this will probably need @timperrett 's eyes.

3 changes: 2 additions & 1 deletion core/src/main/scala/AccessToken.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
package nelson

final case class AccessToken(
value: String
value: String,
isPrivate: Boolean = false
Copy link
Author

Choose a reason for hiding this comment

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

This is because Gitlab treats OAuth and private personal tokens differently.

Copy link
Member

Choose a reason for hiding this comment

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

Should this be a sum type instead?

sealed abstract class Token extends Product with Serializable

object Token {
  final case class PersonalToken(...) extends Token
  final case class OAuth(...) extends Token
}

)
204 changes: 148 additions & 56 deletions core/src/main/scala/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,17 @@ import java.nio.file.{Path, Paths}
import java.security.SecureRandom
import java.security.cert.{CertificateFactory, X509Certificate}
import java.util.concurrent.{ExecutorService, Executors, ScheduledExecutorService, ThreadFactory}

import journal.Logger
import nelson.BannedClientsConfig.HttpUserAgent
import nelson.cleanup.ExpirationPolicy
import nelson.Github.GithubOp
import org.http4s.Uri
import org.http4s.client.Client
import org.http4s.client.blaze._
import storage.StoreOp
import logging.{WorkflowLogger,LoggingOp}
import audit.{Auditor,AuditEvent}
import notifications.{SlackHttp,SlackOp,EmailOp,EmailServer}

import logging.{LoggingOp, WorkflowLogger}
import audit.{AuditEvent, Auditor}
import notifications.{EmailOp, EmailServer, SlackHttp, SlackOp}
import scala.concurrent.ExecutionContext
import scala.concurrent.duration._
import scala.util.control.NonFatal
Expand All @@ -45,57 +44,139 @@ import scheduler.SchedulerOp
import vault._
import vault.http4s._

/**
*
*/
final case class GithubConfig(
domain: Option[String],
clientId: String,
clientSecret: String,
redirectUri: String,
scope: String,
systemAccessToken: AccessToken,
systemUsername: String,
organizationBlacklist: List[String],
organizationAdminList: List[String]
){
sealed abstract class ScmConfig extends Product with Serializable {
def domain: Option[String]
def clientId: String
def clientSecret: String
def redirectUri: String
def scope: String
def systemAccessToken: AccessToken
def systemUsername: String
def organizationBlacklist: List[String]
def organizationAdminList: List[String]
def isEnterprise: Boolean = domain.nonEmpty
def base: String
def oauth: String
def api: String
def tokenEndpoint: String
def loginEndpoint: String
def userEndpoint: String
def userOrgsEndpoint: String
def orgEndpoint(login: String): String
def repoEndpoint(page: Int = 1): String
def webhookEndpoint(slug: Slug): String
def contentsEndpoint(slug: Slug, path: String): String
def releaseEndpoint(slug: Slug, releaseId: String): String

def withOrganizationAdminList(l: List[String]): ScmConfig

private [nelson] def encodeURI(uri: String): String =
java.net.URLEncoder.encode(uri, "UTF-8")
}
object ScmConfig {
final case class GithubConfig(
domain: Option[String],
clientId: String,
clientSecret: String,
redirectUri: String,
scope: String,
systemAccessToken: AccessToken,
systemUsername: String,
organizationBlacklist: List[String],
organizationAdminList: List[String]) extends ScmConfig {

val oauth =
"https://"+ domain.fold("github.com")(identity)
val base: String =
"https://"+ domain.fold("github.com")(identity)

val api =
"https://"+ domain.fold("api.github.com")(_+"/api/v3")
val oauth: String =
base

val tokenEndpoint =
s"${oauth}/login/oauth/access_token"
val api: String =
"https://"+ domain.fold("api.github.com")(_+"/api/v3")

val loginEndpoint =
s"${oauth}/login/oauth/authorize?client_id=${clientId}&redirect_uri=${encodeURI(redirectUri)}&scope=${scope}"
val tokenEndpoint: String =
s"$oauth/login/oauth/access_token"

val userEndpoint =
s"${api}/user"
val loginEndpoint: String =
s"$oauth/login/oauth/authorize?client_id=$clientId&redirect_uri=${encodeURI(redirectUri)}&scope=$scope"

val userOrgsEndpoint =
s"${userEndpoint}/orgs"
val userEndpoint: String =
s"$api/user"

def orgEndpoint(login: String) =
s"${api}/orgs/${login}"
val userOrgsEndpoint: String =
s"$userEndpoint/orgs"

def repoEndpoint(page: Int = 1) =
s"${api}/user/repos?affiliation=owner,organization_member&visibility=all&direction=asc&page=${page}"
def orgEndpoint(login: String): String =
s"$api/orgs/$login"

def webhookEndpoint(slug: Slug) =
s"${api}/repos/${slug}/hooks"
def repoEndpoint(page: Int = 1): String =
s"$api/user/repos?affiliation=owner,organization_member&visibility=all&direction=asc&page=$page"

def contentsEndpoint(slug: Slug, path: String) =
s"${api}/repos/${slug}/contents/${path}"
def webhookEndpoint(slug: Slug): String =
s"$api/repos/$slug/hooks"

def releaseEndpoint(slug: Slug, releaseId: Long) =
s"${api}/repos/${slug}/releases/${releaseId}"
def contentsEndpoint(slug: Slug, path: String): String =
s"$api/repos/$slug/contents/$path"

private [nelson] def encodeURI(uri: String): String =
java.net.URLEncoder.encode(uri, "UTF-8")
def releaseEndpoint(slug: Slug, releaseId: String): String =
s"$api/repos/$slug/releases/$releaseId"

def withOrganizationAdminList(l: List[String]): ScmConfig =
this.copy(organizationAdminList = l)
}

final case class GitlabConfig(
domain: Option[String],
clientId: String,
clientSecret: String,
redirectUri: String,
scope: String,
systemAccessToken: AccessToken,
systemUsername: String,
organizationBlacklist: List[String],
organizationAdminList: List[String]) extends ScmConfig {

val base: String =
"https://" + domain.fold("gitlab.com")(identity)

val oauth: String =
base + "/oauth"

val api: String =
base + "/api/v4"

val tokenEndpoint: String =
s"$oauth/token"

val loginEndpoint: String =
s"$oauth/authorize?client_id=$clientId&redirect_uri=${encodeURI(redirectUri)}&response_type=code"

val userEndpoint: String =
s"$api/user"

val userOrgsEndpoint: String =
s"$api/groups"

def orgEndpoint(login: String): String =
s"$api/groups/$login"

def repoEndpoint(page: Int = 1): String =
s"$api/projects?page=$page"

def webhookEndpoint(slug: Slug): String =
s"$api/projects/${ urlEncode(slug.toString) }/hooks"

def contentsEndpoint(slug: Slug, path: String): String =
s"$api/projects/${ urlEncode(slug.toString) }/repository/files/$path"

def releaseEndpoint(slug: Slug, releaseId: String): String =
s"$api/projects/${ urlEncode(slug.toString) }/repository/tags/$releaseId"

def withOrganizationAdminList(l: List[String]): ScmConfig =
this.copy(organizationAdminList = l)

private[this] def urlEncode(s: String) = java.net.URLEncoder.encode(s, "UTF-8")
}
}

/**
Expand Down Expand Up @@ -316,7 +397,7 @@ final case class PolicyConfig(
* actually cares about.
*/
final case class NelsonConfig(
git: GithubConfig,
git: ScmConfig,
network: NetworkConfig,
security: SecurityConfig,
database: DatabaseConfig,
Expand Down Expand Up @@ -400,7 +481,12 @@ object Config {
val nomadcfg = readNomad(cfg.subconfig("nelson.nomad"))

val gitcfg = readGithub(cfg.subconfig("nelson.github"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is configuration of github and gitlab under nelson.github

Copy link
Author

Choose a reason for hiding this comment

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

yep, for the time being. Which one is used is determined by the param service, e.g.

service = "gitlab"

Default is github anyway.

I tried not to add many breaking changes in this first version of Gitlab support, but I think that we should re-think the algebra and the configuration to be more generic. Probably in another PR if you agree.

val git = new Github.GithubHttp(gitcfg, http0)
val git: GithubOp ~> Task = gitcfg match {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use something other than GithubOp now

Copy link
Author

Choose a reason for hiding this comment

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

Same idea as before. I left that kind of changes for a second review.

case _: ScmConfig.GithubConfig =>
new Github.GithubHttp(gitcfg, http0)
case _ =>
new Gitlab.GitlabHttp(gitcfg, http4sClient(timeout))
Copy link
Member

Choose a reason for hiding this comment

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

Picky but I generally prefer pat mat instead of type casing, if only for theoretical reasons described here: https://typelevel.org/blog/2014/11/10/why_is_adt_pattern_matching_allowed.html

Also perhaps get rid of _ and do an explicit match on GitLab's constructor so any potential future additions error here as they should.

}

val workflowConf = readWorkflowLogger(cfg.subconfig("nelson.workflow-logger"))
val workflowlogger = new WorkflowLogger(
Expand Down Expand Up @@ -754,18 +840,24 @@ object Config {
monitoringPort = cfg.require[Int]("monitoring-port")
)

private def readGithub(cfg: KConfig): GithubConfig =
GithubConfig(
domain = cfg.lookup[String]("domain"),
clientId = cfg.require[String]("client-id"),
clientSecret = cfg.require[String]("client-secret"),
redirectUri = cfg.require[String]("redirect-uri"),
scope = cfg.require[String]("scope"),
systemAccessToken = AccessToken(cfg.require[String]("access-token")),
systemUsername = cfg.require[String]("system-username"),
organizationBlacklist = cfg.lookup[List[String]]("organization-blacklist").getOrElse(Nil),
organizationAdminList = cfg.lookup[List[String]]("organization-admins").getOrElse(Nil)
private def readGithub(cfg: KConfig): ScmConfig = {
val service = cfg.lookup[String]("service").getOrElse("github")
Copy link
Author

Choose a reason for hiding this comment

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

The service configuration param is used to choose the scm interpreter: Gitlab or GIthub (default).

Copy link
Contributor

@kaiserpelagic kaiserpelagic Dec 28, 2017

Choose a reason for hiding this comment

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

For reading configuration for different interpreters I might do something like

kfg.lookup[String]("scm") match {
  case Some("github") => readGithub(kfg.subconfig("github"))
  case Some("gitlab") => readGitlab(kfg.subconfig("gitlab"))
  case _             => None
}

Copy link
Author

@juanjovazquez juanjovazquez Dec 28, 2017

Choose a reason for hiding this comment

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

For the time being it's the same configuration.

val attrs = (
cfg.lookup[String]("domain"),
cfg.require[String]("client-id"),
cfg.require[String]("client-secret"),
cfg.require[String]("redirect-uri"),
cfg.require[String]("scope"),
AccessToken(cfg.require[String]("access-token"), isPrivate = true),
cfg.require[String]("system-username"),
cfg.lookup[List[String]]("organization-blacklist").getOrElse(Nil),
cfg.lookup[List[String]]("organization-admins").getOrElse(Nil)
)
import ScmConfig._
val confBuilder =
if (service == "github") GithubConfig else GitlabConfig
confBuilder.tupled(attrs)
}

private def readSlack(cfg: KConfig): Option[SlackConfig] = {
for {
Expand Down
12 changes: 6 additions & 6 deletions core/src/main/scala/Github.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ object Github {
) extends Event

final case class ReleaseEvent(
id: Long,
id: String,
Copy link
Member

Choose a reason for hiding this comment

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

One trick we can try is parameterize it into ReleaseEvent[A] so we have ReleaseEvent[Long] and ReleaseEvent[String].. wdyt? @timperrett @kaiserpelagic

slug: Slug,
repositoryId: Long
) extends Event

final case class Release(
id: Long,
id: String,
url: String,
htmlUrl: String,
assets: Seq[Asset],
Expand Down Expand Up @@ -129,7 +129,7 @@ object Github {
final case class GetReleaseAssetContent(asset: Github.Asset, t: AccessToken)
extends GithubOp[Github.Asset]

final case class GetRelease(slug: Slug, releaseId: ID, t: AccessToken)
final case class GetRelease(slug: Slug, releaseId: String, t: AccessToken)
extends GithubOp[Github.Release]

final case class GetUserRepositories(token: AccessToken)
Expand Down Expand Up @@ -178,7 +178,7 @@ object Github {
* nelson gets notified of a release, as the payload we get does not contain
* the assets that we need.
*/
def fetchRelease(slug: Slug, id: ID)(t: AccessToken): GithubOpF[Github.Release] =
def fetchRelease(slug: Slug, id: String)(t: AccessToken): GithubOpF[Github.Release] =
for {
r <- Free.liftFC(GetRelease(slug, id, t))
a <- fetchReleaseAssets(r)(t)
Expand Down Expand Up @@ -226,7 +226,7 @@ object Github {
}
}

final class GithubHttp(cfg: GithubConfig, http: dispatch.Http) extends (GithubOp ~> Task) {
final class GithubHttp(cfg: ScmConfig, http: dispatch.Http) extends (GithubOp ~> Task) {
Copy link
Member

Choose a reason for hiding this comment

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

mmmmm this is a bit bothersome. On one hand I don't like using subtypes for data types that are ADTs on the other hand the only config this should be called with is the GithubConfig.. I would learn towards using the subtype anyways.

import java.net.URI
import dispatch._, Defaults._
import nelson.Json._
Expand Down Expand Up @@ -272,7 +272,7 @@ object Github {
b <- http(url(a) OK as.String).toTask
} yield asset.copy(content = Option(b))

case GetRelease(slug: Slug, releaseId: ID, t: AccessToken) =>
case GetRelease(slug: Slug, releaseId: String, t: AccessToken) =>
for {
resp <- fetch(cfg.releaseEndpoint(slug, releaseId), t)
rel <- fromJson[Github.Release](resp)
Expand Down
Loading