Enable https

[ivan-pinatti]

what

• Add support for self-signed TLS certificates
• A few minor improvements.

why

• To have an end-to-end secure connection. e.g. HIPAA compliant.
• This PR is related to the issue Add end-to-end HTTPS support terraform-aws-jenkins#21

[osterman]

Cloud Posse, LLC =)

[osterman]

@ivan-pinatti what are your thoughts on converting this instead to an ARG?

[ivan-pinatti]

@osterman I did it for testing purposes on my environment, however, AWS CodeBuild is running an older version of Docker that doesn't support it. So, for now we can't change.

I already submitted a PR to AWS CodeBuild adding a newer Docker Image version of Ubuntu and on top of that a newer version of Docker.

The problem is that I submitted both more than a month ago and I did not receive any interaction from AWS, it seems that they are not much attentive to the repo.

[osterman]

Oh wow, that's crazy! Feels like this feature has been in docker for ages now.

[osterman]

Oh, do you mean that they don't support multi-line ARG? I see you're using it everywhere else. Could you add a link to your PR?

[ivan-pinatti]

@osterman I was referring to the Jenkins version itself, in the first lines of the Dockerfile, ex:

# set default version if no argument was provided
ARG JENKINS_VERSION="2.103"
FROM jenkins/jenkins:${JENKINS_VERSION}-slim

This is not supported yet.

Regarding to pass the package list as argument I don't see any gain on doing it at the moment.

[osterman]

Regarding to pass the package list as argument I don't see any gain on doing it at the moment.

personal bias is in favor of reducing the inline bash, when the dockerfile language defines a convention for this.

[ivan-pinatti]

You mean, you prefer this:

# install required packages
RUN apk update && apk upgrade && apk add --no-cache bash=4.4.12-r2 docker=17.10.0-r0 gettext=0.19.8.1-r1 git=2.15.0-r1 make=4.2.1-r0 openssh=7.5_p1-r8 openssl=1.0.2n-r0

Instead of this:

# install required packages
RUN readonly PACKAGES=" \
      bash=4.4.12-r2 \
      docker=17.10.0-r0 \
      gettext=0.19.8.1-r1 \
      git=2.15.0-r1 \
      make=4.2.1-r0 \
      openssh=7.5_p1-r8 \
      openssl=1.0.2n-r0 \
    " \
    && apk update \
    && apk upgrade \
    && apk add --no-cache ${PACKAGES}

?

[osterman]

More like:

ARG PACKAGES="bash=4.4.12-r2 docker=17.10.0-r0 gettext=0.19.8.1-r1 git=2.15.0-r1 make=4.2.1-r0 openssh=7.5_p1-r8 openssl=1.0.2n-r0"
RUN apk update && apk upgrade && apk add --no-cache ${PACKAGES} ...

[ivan-pinatti]

Considering this as integrated part of Terraform Jenkins AWS, we would have to add it as a parameter into AWS CodeBuild code section as well, and I see no advantage in doing it. At least not with the current solution design.

Even though, I would prefer to have it as multi-line for better readability and improved code maintenance, something like:

# install required packages
ARG PACKAGES=" \
      bash=4.4.12-r2 \
      docker=17.10.0-r0 \
      gettext=0.19.8.1-r1 \
      git=2.15.0-r1 \
      make=4.2.1-r0 \
      openssh=7.5_p1-r8 \
      openssl=1.0.2n-r0 \
    " \

RUN apk update \
    && apk upgrade \
    && apk add --no-cache ${PACKAGES}

However, I'm not sure that Docker supports ARG multi-line syntax yet, as seen here:
(https://github.com/moby/moby/issues/35950)[https://github.com/moby/moby/issues/35950]

[osterman]

Looks good to me. Two small nit picks.

[osterman]

@ivan-pinatti btw, check out the updated description. Generally, we use this convention:

## what
* thing that changed

## why
* justifications for change

[ivan-pinatti]

@osterman all changes done.

[aknysh]

@ivan-pinatti
this PR depends on cloudposse/terraform-aws-codebuild#14
(Jenkins ENV vars).

Please se inline comments.

Basically, we don't want to pollute the codebuild module` with application-specific settings, in this case Jenkins-related

[ivan-pinatti]

What is your proposition then @aknysh ?

Because the URL is required to generate the self-signed certificate during the build.

[aknysh]

@ivan-pinatti
thank you again for your contribution.
I'll review it little bit later, we should come up with a good way to solve the issue.

[ivan-pinatti]

No problem @aknysh.

I reviewed and I think this is the only way to give the user the option (via parameter in the module) to use their own URL and to make Jenkins to auto-generate the self-signed certificate.

The other possible solution would be somehow manage to change the Jenkins build process to retrieve a valid certificate from somewhere else, perhaps AWS S3 for example.

[aknysh]

@ivan-pinatti
thanks
it was busy few weeks for us, but I'll get to this shortly and we'll discuss the best solution

[ivan-pinatti]

@aknysh no problem, take your time. :)

This is a new feature, no need to rush it.

[kmcquade]

@aknysh @ivan-pinatti any plans to address this? I know it's been a while.

[kmcquade]

@aknysh friendly reminder :)