Code review comment for ~bryce/ubuntu-docker-images/+git/bind9:initial-implementation

On Thu, Sep 09, 2021 at 08:10:25PM -0000, Sergio Durigan Junior wrote:
> Review: Approve
>
> Thanks, Bryce.
>
> Re. the copyright holder, I think that's the right thing to do but IANAL. I'll talk to Valentin, because some of our other images have the same problem. Meanwhile, could you please remove the LICENSE file from the commit? I'm not even sure if we want to license this work using BSD-3-Clause or if we'll use another license...
>

Sure. Looks like this was just inherited from deriving off memcached.
Looking at some of the other OCI files, looks like this generally is
just the license of the packaged software itself. For bind9 that
appears to be MPLv2.0. In some cases where the OCI image derives from a
pre-existing docker image's git repo that might make sense but leaves
inconsistency across the packages.

It'll be good to get Valentin's direction, this isn't something I'd be
comfortable deciding either. But having a policy established would be
of value going forward.

> As for the other changes, they look good now modulo a few nits. I wasn't able to build the Impish version of the image, but I did build the Hirsute one and it looks fine. Looking forward to running the unit tests for it.
>
> +1, LGTM.

Thanks. I've added the suggested changes and will land to the main
repo.

Bryce

> Diff comments:
>
> > diff --git a/Dockerfile b/Dockerfile
> > new file mode 100644
> > index 0000000..1d001d6
> > --- /dev/null
> > +++ b/Dockerfile
> > @@ -0,0 +1,30 @@
> > +FROM ubuntu:impish
> > +
> > +ENV TZ UTC
> > +ENV BIND_USER bind
>
> No need for BIND_USER here.
>
> > +
> > +RUN set -eux; \
> > +# installation
> > + apt-get update; \
> > + DEBIAN_FRONTEND=noninteractive apt-get full-upgrade -y; \
> > + DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
> > + bind9 \
> > + bind9-utils \
> > + tzdata \
> > + ; \
> > + DEBIAN_FRONTEND=noninteractive apt-get remove --purge --auto-remove -y; \
> > + rm -rf /var/lib/apt/lists/*; \
> > +# smoke test
> > + named -v; \
> > +# create manifest
> > + mkdir -p /usr/share/rocks; \
> > + (echo "# os-release" && cat /etc/os-release && echo "# dpkg-query" && dpkg-query -f '${db:Status-Abbrev},${binary:Package},${Version},${source:Package},${Source:Version}\n' -W) > /usr/share/rocks/dpkg.query
> > +
> > +USER bind
> > +VOLUME ["/var/cache/bind", "/var/lib/bind"]
> > +
> > +EXPOSE 53
> > +
> > +COPY docker-entrypoint.sh /usr/local/bin/
> > +ENTRYPOINT ["docker-entrypoint.sh"]
> > +CMD docker-entrypoint.sh
>
> You don't need the CMD statement here.
>
> > diff --git a/data/bind9.yaml b/data/bind9.yaml
> > new file mode 100644
> > index 0000000..45b6dfc
> > --- /dev/null
> > +++ b/data/bind9.yaml
> > @@ -0,0 +1,34 @@
> > +application: Bind9
> > +main: true
> > +repo: bind9
> > +description: >
> > + BIND 9 provides software for Domain Name System (DNS) management including both defining domain names authoritatively for a given DNS zone, and recursively resolving domain names to their IP addresses.
> > + In addition to BIND 9's DNS server itself, `named`, this container also includes tools for performing DNS queries and dynamic updates.
> > + Read more on the [BIND 9 website](https://www.isc.org/bind/).
> > +version: "9.16.15"
> > +base: Ubuntu 21.10
> > +architectures:
> > + - amd64
> > + - arm64
> > + - ppc64el
> > + - s390x
> > +docker:
> > + parameters:
> > + - -p 30053:53
> > + access: Access your Bind9 server at `localhost:30053`
> > +parameters:
>
> I think it's worth mentioning the two volumes being explicitly exported in the Dockerfile: /var/lib/bind and /var/cache/bind.
>
> > + - type: -e
> > + value: TZ=UTC
> > + description: Timezone.
> > + - type: -p
> > + value: 30053:53
> > + description: Expose bind9 on `localhost:30053`.
> > + - type: -v
> > + value: /path/to/bind/configuration:/etc/bind/named.conf
> > + description: Local [configuration file](https://bind9.readthedocs.io/en/latest/reference.html) `named.conf` (try [this example](https://git.launchpad.net/~canonical-server/ubuntu-docker-images/+git/bind9/plain/examples/caching-nameserver/named.conf.options)).
> > +debug:
> > + - nothing
> > +microk8s:
> > + deploy:
> > + link: https://git.launchpad.net/~canonical-server/ubuntu-docker-images/+git/bind9/plain/examples/bind9-deployment.yml
> > + access: You will now be able to connect to the Bind9 server on localhost:30053`.
> > diff --git a/examples/caching-nameserver/named.conf.options b/examples/caching-nameserver/named.conf.options
> > new file mode 100644
> > index 0000000..1b03e16
> > --- /dev/null
> > +++ b/examples/caching-nameserver/named.conf.options
> > @@ -0,0 +1,26 @@
> > +options {
> > + directory "/var/cache/bind";
> > +
> > + // If there is a firewall between you and nameservers you want
> > + // to talk to, you may need to fix the firewall to allow multiple
> > + // ports to talk. See http://www.kb.cert.org/vuls/id/800113
> > +
> > + // If your ISP provided one or more IP addresses for stable
> > + // nameservers, you probably want to use them as forwarders.
> > + // Uncomment the following block, and insert the addresses replacing
> > + // the all-0's placeholder.
> > +
> > + // Set the IP addresses of your ISP's DNS servers:
> > + forwarders {
> > + 1.2.3.4;
> > + 5.6.7.8;
>
> These forwarders are phony; were you able to use this file when testing the microk8s stuff? Maybe it's better to comment out this block?
>
> > + };
> > +
> > + //========================================================================
> > + // If BIND logs error messages about the root key being expired,
> > + // you will need to update your keys. See https://www.isc.org/bind-keys
> > + //========================================================================
> > + dnssec-validation auto;
> > +
> > + listen-on-v6 { any; };
> > +};
>
>
> --
> https://code.launchpad.net/~bryce/ubuntu-docker-images/+git/bind9/+merge/408328
> You are the owner of ~bryce/ubuntu-docker-images/+git/bind9:initial-implementation.