This builds a container image containing the DCO checking script.
It is then published at:

   registry.gitlab.com/libvirt/libvirt-jenkins-ci/check-dco:master

from where it can be used in all other project repositories. This
avoids having to copy the check-dco.py script into every repo
when we want to make changes to it.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 .gitlab-ci.yml         | 39 +++++++++++++++++
 check-dco/Dockerfile   |  6 +++
 check-dco/check-dco.py | 99 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+)
 create mode 100644 .gitlab-ci.yml
 create mode 100644 check-dco/Dockerfile
 create mode 100755 check-dco/check-dco.py

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 0000000..83b0d64
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,39 @@
+
+stages:
+  - containers
+  - postbuild
+
+.build_container_template: &build_container_definition
+  image: docker:stable
+  stage: containers
+  services:
+    - docker:dind
+  before_script:
+    - docker info
+    - docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD}
+  script:
+    - docker build --tag ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG} -f ${NAME}/Dockerfile ${NAME}
+    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
+  after_script:
+    - docker logout
+
+build-dco:
+  <<: *build_container_definition
+  variables:
+    NAME: check-dco
+
+# Check that all commits are signed-off for the DCO. Skip
+# on master branch and -maint branches, since we only need
+# to test developer's personal branches.
+check-dco:
+  # In most projects we'd put this in a "prebuild", but this
+  # repo is a special case as we need to actually build the
+  # container first!
+  stage: postbuild
+  image: ${CI_REGISTRY_IMAGE}/check-dco:${CI_COMMIT_REF_SLUG}
+  script:
+    - /check-dco.py
+  only:
+    - branches
+  except:
+    - master
diff --git a/check-dco/Dockerfile b/check-dco/Dockerfile
new file mode 100644
index 0000000..86cada7
--- /dev/null
+++ b/check-dco/Dockerfile
@@ -0,0 +1,6 @@
+FROM centos:8
+
+RUN dnf -y install python3 git && \
+    dnf -y clean all
+
+COPY check-dco.py check-dco.py
diff --git a/check-dco/check-dco.py b/check-dco/check-dco.py
new file mode 100755
index 0000000..f8204ca
--- /dev/null
+++ b/check-dco/check-dco.py
@@ -0,0 +1,99 @@
+#!/usr/bin/env python3
+#
+# ci-job.py: validate all commits are signed off
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library.  If not, see
+# <http://www.gnu.org/licenses/>.
+
+import os
+import os.path
+import sys
+import subprocess
+
+cwd = os.getcwd()
+reponame = os.path.basename(cwd)
+repourl = "https://gitlab.com/libvirt/%s.git" % reponame
+
+subprocess.check_call(["git", "remote", "add", "dcocheck", repourl])
+subprocess.check_call(["git", "fetch", "dcocheck", "master"],
+                      stdout=subprocess.DEVNULL,
+                      stderr=subprocess.DEVNULL)
+
+ancestor = subprocess.check_output(["git", "merge-base", "dcocheck/master", "HEAD"],
+                                   universal_newlines=True)
+
+ancestor = ancestor.strip()
+
+subprocess.check_call(["git", "remote", "rm", "dcocheck"])
+
+errors = False
+
+print("\nChecking for 'Signed-off-by: NAME <EMAIL>' on all commits since %s...\n" % ancestor)
+
+log = subprocess.check_output(["git", "log", "--format=%H %s", ancestor + "..."],
+                              universal_newlines=True)
+
+if log == "":
+    commits = []
+else:
+    commits = [[c[0:40], c[41:]] for c in log.strip().split("\n")]
+
+for sha, subject in commits:
+
+    msg = subprocess.check_output(["git", "show", "-s", sha],
+                                  universal_newlines=True)
+    lines = msg.strip().split("\n")
+
+    print("🔍 %s %s" % (sha, subject))
+    sob = False
+    for line in lines:
+        if "Signed-off-by:" in line:
+            sob = True
+            if "localhost" in line:
+                print("    ❌ FAIL: bad email in %s" % line)
+                errors = True
+
+    if not sob:
+        print("    ❌ FAIL missing Signed-off-by tag")
+        errors = True
+
+if errors:
+    print("""
+
+❌ ERROR: One or more commits are missing a valid Signed-off-By tag.
+
+
+This project requires all contributors to assert that their contributions
+are provided in compliance with the terms of the Developer's Certificate
+of Origin 1.1 (DCO):
+
+  https://developercertificate.org/
+
+To indicate acceptance of the DCO every commit must have a tag
+
+  Signed-off-by: REAL NAME <EMAIL>
+
+This can be achieved by passing the "-s" flag to the "git commit" command.
+
+To bulk update all commits on current branch "git rebase" can be used:
+
+  git rebase -i master -x 'git commit --amend --no-edit -s'
+
+""")
+
+    sys.exit(1)
+
+sys.exit(0)
-- 
2.24.1

On Wed, 2020-04-08 at 12:54 +0100, Daniel P. Berrangé wrote:
> This builds a container image containing the DCO checking script.
> It is then published at:
> 
>    registry.gitlab.com/libvirt/libvirt-jenkins-ci/check-dco:master
> 
> from where it can be used in all other project repositories. This
> avoids having to copy the check-dco.py script into every repo
> when we want to make changes to it.

So the idea is that the current dco job in, for example, libvirt,
will become

  dco:
    stage: prebuild
    script:
      - /check-dco.py
    image: registry.gitlab.com/libvirt/libvirt-jenkins-ci/check-dco:master

and the same job will be added for all other projects, correct?

> +.build_container_template: &build_container_definition
> +  image: docker:stable
> +  stage: containers
> +  services:
> +    - docker:dind

TIL: dind means "Docker in Docker".

> +  before_script:
> +    - docker info
> +    - docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD}

For those following along at home, these variables are set
automatically by GitLab:

  https://gitlab.com/help/ci/variables/predefined_variables.md

> +  script:
> +    - docker build --tag ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG} -f ${NAME}/Dockerfile ${NAME}

The '-f ${NAME}/Dockerfile' part is unnecessary, because Dockerfile
is the default name 'docker build' looks for and it will already
enter the directory you provided (it wouldn't find the script
otherwise).

> +    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}

I'm not super happy about returning to the image:master naming
convention, because most tooling around containers will expect the
tag name to be 'latest' and we had just recently managed to adopt it
throughout, but of course we need to make sure containers build off
branches do not overwrite the known good image built from master...

A more important question is whether this should live in
libvirt-jenkins-ci at all. In the long run the Jenkins stuff is going
to be dropped, and that would lead to the already pretty awkward name
becoming completely inaccurate.

libvirt-dockerfiles would be an obvious candidate right now, but as
we start moving generated Dockerfiles to the corresponding
repositories that's going to be abandoned and eventually removed.

I was going to suggest that, once the Jenkins stuff has been removed,
we could rename the repository to lcitool, but maybe we should call
it libvirt-ci instead? Or even just ci for that matter: we already
have a number of non-prefixed repositories, and if they contain
internal tools rather than projects that are intended to be released
and distributed I think that's perfectly fine and if anything
highlights the distinction.

> +++ b/check-dco/Dockerfile
> @@ -0,0 +1,6 @@
> +FROM centos:8
> +
> +RUN dnf -y install python3 git && \
> +    dnf -y clean all
> +
> +COPY check-dco.py check-dco.py

I suggest using an absolute path for the destination for clarity,
though it shouldn't actually make a difference.

Can we call the script "check-dco" without the .py suffix? The
language used is an implementation detail that users will not be
interested in.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Apr 08, 2020 at 04:21:31PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-08 at 12:54 +0100, Daniel P. Berrangé wrote:
> > This builds a container image containing the DCO checking script.
> > It is then published at:
> > 
> >    registry.gitlab.com/libvirt/libvirt-jenkins-ci/check-dco:master
> > 
> > from where it can be used in all other project repositories. This
> > avoids having to copy the check-dco.py script into every repo
> > when we want to make changes to it.
> 
> So the idea is that the current dco job in, for example, libvirt,
> will become
> 
>   dco:
>     stage: prebuild
>     script:
>       - /check-dco.py
>     image: registry.gitlab.com/libvirt/libvirt-jenkins-ci/check-dco:master
> 
> and the same job will be added for all other projects, correct?

Yeah, exactly.

> 
> > +.build_container_template: &build_container_definition
> > +  image: docker:stable
> > +  stage: containers
> > +  services:
> > +    - docker:dind
> 
> TIL: dind means "Docker in Docker".
> 
> > +  before_script:
> > +    - docker info
> > +    - docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD}
> 
> For those following along at home, these variables are set
> automatically by GitLab:
> 
>   https://gitlab.com/help/ci/variables/predefined_variables.md
> 
> > +  script:
> > +    - docker build --tag ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG} -f ${NAME}/Dockerfile ${NAME}
> 
> The '-f ${NAME}/Dockerfile' part is unnecessary, because Dockerfile
> is the default name 'docker build' looks for and it will already
> enter the directory you provided (it wouldn't find the script
> otherwise).

Oh yes.

> > +    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
> 
> I'm not super happy about returning to the image:master naming
> convention, because most tooling around containers will expect the
> tag name to be 'latest' and we had just recently managed to adopt it
> throughout, but of course we need to make sure containers build off
> branches do not overwrite the known good image built from master...

We could have conditional logic that changes it to "latest" if
$CI_COMMIT_REF_SLUG == "master"

> I was going to suggest that, once the Jenkins stuff has been removed,
> we could rename the repository to lcitool, but maybe we should call
> it libvirt-ci instead? Or even just ci for that matter: we already
> have a number of non-prefixed repositories, and if they contain
> internal tools rather than projects that are intended to be released
> and distributed I think that's perfectly fine and if anything
> highlights the distinction.

Yeah, I was about to suggest that we just rename this to "libvirt-ci",
and accept the short term pain for people with broken URLs.

It is highly desirable to avoid very short, generic names like "ci",
because when you fork a repo into your own namespace, the repo names
must match and thus you risk a clash if you've forked a repo called
"ci" from an unrelated project.


> 
> > +++ b/check-dco/Dockerfile
> > @@ -0,0 +1,6 @@
> > +FROM centos:8
> > +
> > +RUN dnf -y install python3 git && \
> > +    dnf -y clean all
> > +
> > +COPY check-dco.py check-dco.py
> 
> I suggest using an absolute path for the destination for clarity,
> though it shouldn't actually make a difference.
> 
> Can we call the script "check-dco" without the .py suffix? The
> language used is an implementation detail that users will not be
> interested in.

Sure, we can drop the .py

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wed, 2020-04-08 at 15:26 +0100, Daniel P. Berrangé wrote:
> On Wed, Apr 08, 2020 at 04:21:31PM +0200, Andrea Bolognani wrote:
> > On Wed, 2020-04-08 at 12:54 +0100, Daniel P. Berrangé wrote:
> > > +    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
> > 
> > I'm not super happy about returning to the image:master naming
> > convention, because most tooling around containers will expect the
> > tag name to be 'latest' and we had just recently managed to adopt it
> > throughout, but of course we need to make sure containers build off
> > branches do not overwrite the known good image built from master...
> 
> We could have conditional logic that changes it to "latest" if
> $CI_COMMIT_REF_SLUG == "master"

If that logic doesn't end up being too complicated and hard to
maintain, I would certainly like that.

> > I was going to suggest that, once the Jenkins stuff has been removed,
> > we could rename the repository to lcitool, but maybe we should call
> > it libvirt-ci instead? Or even just ci for that matter: we already
> > have a number of non-prefixed repositories, and if they contain
> > internal tools rather than projects that are intended to be released
> > and distributed I think that's perfectly fine and if anything
> > highlights the distinction.
> 
> Yeah, I was about to suggest that we just rename this to "libvirt-ci",
> and accept the short term pain for people with broken URLs.
> 
> It is highly desirable to avoid very short, generic names like "ci",
> because when you fork a repo into your own namespace, the repo names
> must match and thus you risk a clash if you've forked a repo called
> "ci" from an unrelated project.

I think that boat has sailed already... If you look, for example, at

  https://github.com/kubernetes/
  https://github.com/kubevirt/
  https://github.com/kata-containers/

you'll see that a lot of the repositories living in those
organizations have pretty generic names, so much so that you can
already spot conflicts at first glance, plus as I said we have
stuff like

  https://gitlab.com/libvirt/hooks
  https://gitlab.com/libvirt/scripts

in our own organization already, so I wouldn't worry too much about
this.

Personally, what I usually do when cloning such a repository is
rename the clone of $org/$repo to $org-$repo right away and then I
never have to even think about it again. It's really not a big deal.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Apr 08, 2020 at 04:54:17PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-08 at 15:26 +0100, Daniel P. Berrangé wrote:
> > On Wed, Apr 08, 2020 at 04:21:31PM +0200, Andrea Bolognani wrote:
> > > On Wed, 2020-04-08 at 12:54 +0100, Daniel P. Berrangé wrote:
> > > > +    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
> > > 
> > > I'm not super happy about returning to the image:master naming
> > > convention, because most tooling around containers will expect the
> > > tag name to be 'latest' and we had just recently managed to adopt it
> > > throughout, but of course we need to make sure containers build off
> > > branches do not overwrite the known good image built from master...
> > 
> > We could have conditional logic that changes it to "latest" if
> > $CI_COMMIT_REF_SLUG == "master"
> 
> If that logic doesn't end up being too complicated and hard to
> maintain, I would certainly like that.
> 
> > > I was going to suggest that, once the Jenkins stuff has been removed,
> > > we could rename the repository to lcitool, but maybe we should call
> > > it libvirt-ci instead? Or even just ci for that matter: we already
> > > have a number of non-prefixed repositories, and if they contain
> > > internal tools rather than projects that are intended to be released
> > > and distributed I think that's perfectly fine and if anything
> > > highlights the distinction.
> > 
> > Yeah, I was about to suggest that we just rename this to "libvirt-ci",
> > and accept the short term pain for people with broken URLs.
> > 
> > It is highly desirable to avoid very short, generic names like "ci",
> > because when you fork a repo into your own namespace, the repo names
> > must match and thus you risk a clash if you've forked a repo called
> > "ci" from an unrelated project.
> 
> I think that boat has sailed already... If you look, for example, at
> 
>   https://github.com/kubernetes/
>   https://github.com/kubevirt/
>   https://github.com/kata-containers/
> 
> you'll see that a lot of the repositories living in those
> organizations have pretty generic names, so much so that you can
> already spot conflicts at first glance, plus as I said we have

Since they clearly have issues, we shouldn't follow their bad
example.

> stuff like
> 
>   https://gitlab.com/libvirt/hooks
>   https://gitlab.com/libvirt/scripts
> 
> in our own organization already, so I wouldn't worry too much about
> this.

I consider those few cases in libvirt to be a mistake, but since
we don't actively use those repos I didn't care about them.

This naming clash has hit me several times, so I don't want to
make it worse by creating super generic names for libvirt repos.

> Personally, what I usually do when cloning such a repository is
> rename the clone of $org/$repo to $org-$repo right away and then I
> never have to even think about it again. It's really not a big deal.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wed, 2020-04-08 at 16:03 +0100, Daniel P. Berrangé wrote:
> This naming clash has hit me several times, so I don't want to
> make it worse by creating super generic names for libvirt repos.

I clearly don't feel the same way, but I also don't feel strongly
enough about the topic to keep arguing :) So let's rename the
libvirt-jenkins-ci repo to libvirt-ci and then merge this patch
with the few tweaks mentioned earlier.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Apr 08, 2020 at 04:54:17PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-08 at 15:26 +0100, Daniel P. Berrangé wrote:
> > On Wed, Apr 08, 2020 at 04:21:31PM +0200, Andrea Bolognani wrote:
> > > On Wed, 2020-04-08 at 12:54 +0100, Daniel P. Berrangé wrote:
> > > > +    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
> > > 
> > > I'm not super happy about returning to the image:master naming
> > > convention, because most tooling around containers will expect the
> > > tag name to be 'latest' and we had just recently managed to adopt it
> > > throughout, but of course we need to make sure containers build off
> > > branches do not overwrite the known good image built from master...
> > 
> > We could have conditional logic that changes it to "latest" if
> > $CI_COMMIT_REF_SLUG == "master"
> 
> If that logic doesn't end up being too complicated and hard to
> maintain, I would certainly like that.

It doesn't appear possible todo this after all. I can use shell
magic to pick "latest" in the script: section, but I need the
same logic later for the "image:" tag in the "check-dco" job,
and there's no way to achieve that AFAICT.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Thu, 2020-04-09 at 17:44 +0100, Daniel P. Berrangé wrote:
> On Wed, Apr 08, 2020 at 04:54:17PM +0200, Andrea Bolognani wrote:
> > On Wed, 2020-04-08 at 15:26 +0100, Daniel P. Berrangé wrote:
> > > On Wed, Apr 08, 2020 at 04:21:31PM +0200, Andrea Bolognani wrote:
> > > > On Wed, 2020-04-08 at 12:54 +0100, Daniel P. Berrangé wrote:
> > > > > +    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
> > > > 
> > > > I'm not super happy about returning to the image:master naming
> > > > convention, because most tooling around containers will expect the
> > > > tag name to be 'latest' and we had just recently managed to adopt it
> > > > throughout, but of course we need to make sure containers build off
> > > > branches do not overwrite the known good image built from master...
> > > 
> > > We could have conditional logic that changes it to "latest" if
> > > $CI_COMMIT_REF_SLUG == "master"
> > 
> > If that logic doesn't end up being too complicated and hard to
> > maintain, I would certainly like that.
> 
> It doesn't appear possible todo this after all. I can use shell
> magic to pick "latest" in the script: section, but I need the
> same logic later for the "image:" tag in the "check-dco" job,
> and there's no way to achieve that AFAICT.

Yeah, well, whatever. I'll live with it :)

Please post a v2 incorporating the other changes and I'll ACK it.

-- 
Andrea Bolognani / Red Hat / Virtualization