Verify all flag usage does not use _

.travis.yml

@@ -22,6 +22,7 @@ script:
   - ./hack/verify-generated-docs.sh
   - ./hack/verify-swagger-spec.sh
   - ./hack/verify-linkcheck.sh
+  - ./hack/verify-flags-underscore.py
 
 notifications:
   irc: "chat.freenode.net#kubernetes-dev"

hack/verify-flags-underscore.py

+#!/usr/bin/env python
+
+# Copyright 2015 The Kubernetes Authors 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.
+
+from __future__ import print_function
+
+import json
+import mmap
+import os
+import re
+import sys
+import argparse
+
+parser = argparse.ArgumentParser()
+parser.add_argument("filenames", help="list of files to check, all files if unspecified", nargs='*')
+parser.add_argument("-e", "--skip-exceptions", help="ignore hack/verify-flags/exceptions.txt and print all output", action="store_true")
+args = parser.parse_args()
+
+
+dashRE = re.compile('[-_]')
+
+# Cargo culted from http://stackoverflow.com/questions/898669/how-can-i-detect-if-a-file-is-binary-non-text-in-python
+def is_binary(pathname):
+    """Return true if the given filename is binary.
+    @raise EnvironmentError: if the file does not exist or cannot be accessed.
+    @attention: found @ http://bytes.com/topic/python/answers/21222-determine-file-type-binary-text on 6/08/2010
+    @author: Trent Mick <TrentM@ActiveState.com>
+    @author: Jorge Orpinel <jorge@orpinel.com>"""
+    try:
+        f = open(pathname, 'r')
+        CHUNKSIZE = 1024
+        while 1:
+            chunk = f.read(CHUNKSIZE)
+            if '\0' in chunk: # found null byte
+                return True
+            if len(chunk) < CHUNKSIZE:
+                break # done
+    except:
+        return True
+    finally:
+        f.close()
+    return False
+
+def get_all_files(rootdir):
+    all_files = []
+    for root, dirs, files in os.walk(rootdir):
+        # don't visit certain dirs
+        if 'Godeps' in dirs:
+            dirs.remove('Godeps')
+        if 'third_party' in dirs:
+            dirs.remove('third_party')
+        if '.git' in dirs:
+            dirs.remove('.git')
+        if 'exceptions.txt' in files:
+            files.remove('exceptions.txt')
+        if 'known-flags.txt' in files:
+            files.remove('known-flags.txt')
+
+        for name in files:
+            if name.endswith(".svg"):
+                continue
+            if name.endswith(".gliffy"):
+                continue
+            pathname = os.path.join(root, name)
+            if is_binary(pathname):
+                continue
+            all_files.append(pathname)
+    return all_files
+
+def normalize_files(rootdir, files):
+    newfiles = []
+    a = ['Godeps', 'third_party', 'exceptions.txt', 'known-flags.txt']
+    for f in files:
+        if any(x in f for x in a):
+            continue
+        if f.endswith(".svg"):
+            continue
+        if f.endswith(".gliffy"):
+            continue
+        newfiles.append(f)
+    for i, f in enumerate(newfiles):
+        if not os.path.isabs(f):
+            newfiles[i] = os.path.join(rootdir, f)
+    return newfiles
+
+def line_has_bad_flag(line, flagre):
+    m  = flagre.search(line)
+    if not m:
+        return False
+    if "_" in m.group(0):
+        return True
+    return False
+
+# The list of files might not be the whole repo. If someone only changed a
+# couple of files we don't want to run all of the golang files looking for
+# flags. Instead load the list of flags from hack/verify-flags/known-flags.txt
+# If running the golang files finds a new flag not in that file, return an
+# error and tell the user to add the flag to the flag list.
+def get_flags(rootdir, files):
+    # use a set for uniqueness
+    flags = set()
+
+    # preload the 'known' flags
+    pathname = os.path.join(rootdir, "hack/verify-flags/known-flags.txt")
+    f = open(pathname, 'r')
+    for line in f.read().splitlines():
+        flags.add(line)
+    f.close()
+
+    regexs = [ re.compile('Var[P]?\([^,]*, "([^"]*)"'),
+               re.compile('.String[P]?\("([^"]*)",[^,]+,[^)]+\)'),
+               re.compile('.Int[P]?\("([^"]*)",[^,]+,[^)]+\)'),
+               re.compile('.Bool[P]?\("([^"]*)",[^,]+,[^)]+\)'),
+               re.compile('.Duration[P]?\("([^"]*)",[^,]+,[^)]+\)'),
+               re.compile('.StringSlice[P]?\("([^"]*)",[^,]+,[^)]+\)') ]
+
+    new_flags = set()
+    # walk all the files looking for any flags being declared
+    for pathname in files:
+        if not pathname.endswith(".go"):
+            continue
+        f = open(pathname, 'r')
+        data = f.read()
+        f.close()
+        matches = []
+        for regex in regexs:
+            matches = matches + regex.findall(data)
+        for flag in matches:
+            # if the flag doesn't have a - or _ it is not interesting
+            if not dashRE.search(flag):
+                continue
+            if flag not in flags:
+                new_flags.add(flag)
+    if len(new_flags) != 0:
+        print("Found flags in golang files not in the list of known flags. Please add these to hack/verify-flags/known-flags.txt")
+        print("%s" % "\n".join(new_flags))
+        sys.exit(1)
+    return list(flags)
+
+def flags_to_re(flags):
+    """turn the list of all flags we found into a regex find both - and _ version"""
+    flagREs = []
+    for flag in flags:
+        # turn all flag names into regexs which will find both types
+        newre = dashRE.sub('[-_]', flag)
+        flagREs.append(newre)
+    # turn that list of regex strings into a single large RE
+    flagRE = "|".join(flagREs)
+    flagRE = re.compile(flagRE)
+    return flagRE
+
+def load_exceptions(rootdir):
+    exceptions = set()
+    if args.skip_exceptions:
+        return exceptions
+    exception_filename = os.path.join(rootdir, "hack/verify-flags/exceptions.txt")
+    exception_file = open(exception_filename, 'r')
+    for exception in exception_file.read().splitlines():
+        out = exception.split(":", 1)
+        if len(out) != 2:
+            printf("Invalid line in exceptions file: %s" % exception)
+            continue
+        filename = out[0]
+        line = out[1]
+        exceptions.add((filename, line))
+    return exceptions
+
+def main():
+    rootdir = os.path.dirname(__file__) + "/../"
+    rootdir = os.path.abspath(rootdir)
+
+    exceptions = load_exceptions(rootdir)
+
+    if len(args.filenames) > 0:
+        files = args.filenames
+    else:
+        files = get_all_files(rootdir)
+    files = normalize_files(rootdir, files)
+
+    flags = get_flags(rootdir, files)
+    flagRE = flags_to_re(flags)
+
+    bad_lines = []
+    # walk all the file looking for any flag that was declared and now has an _
+    for pathname in files:
+        relname = os.path.relpath(pathname, rootdir)
+        f = open(pathname, 'r')
+        for line in f.read().splitlines():
+            if line_has_bad_flag(line, flagRE):
+                if (relname, line) not in exceptions:
+                    bad_lines.append((relname, line))
+        f.close()
+
+    if len(bad_lines) != 0:
+        if not args.skip_exceptions:
+            print("Found illegal 'flag' usage. If this is a false positive add the following line(s) to hack/verify-flags/exceptions.txt:")
+        for (relname, line) in bad_lines:
+            print("%s:%s" % (relname, line))
+
+if __name__ == "__main__":
+  sys.exit(main())

hack/verify-flags/known-flags.txt

+accept-hosts
+accept-paths
+admission-control
+admission-control-config-file
+advertise-address
+advertised-address
+algorithm-provider
+all-namespaces
+allocate-node-cidrs
+allow-privileged
+api-burst
+api-prefix
+api-rate
+api-servers
+api-token
+api-version
+authorization-mode
+authorization-policy-file
+auth-path
+basic-auth-file
+bench-pods
+bench-quiet
+bench-tasks
+bench-workers
+bind-address
+bind-pods-burst
+bind-pods-qps
+cadvisor-port
+cert-dir
+certificate-authority
+cgroup-prefix
+cgroup-root
+chaos-chance
+check_version_skew
+client-ca-file
+client-certificate
+client-key
+cloud-config
+cloud-provider
+cluster-cidr
+cluster-dns
+cluster-domain
+cluster-name
+cluster-tag
+concurrent-endpoint-syncs
+concurrent_rc_syncs
+configure-cbr0
+container-port
+container-runtime
+cors-allowed-origins
+create-external-load-balancer
+current-release-pr
+current-replicas
+default-container-cpu-limit
+default-container-mem-limit
+deleting-pods-burst
+deleting-pods-qps
+deployment-label-key
+dest-file
+disable-filter
+docker-endpoint
+docker-exec-handler
+driver-port
+dry-run
+dry-run
+dry-run
+dry-run
+duration-sec
+e2e-output-dir
+enable-debugging-handlers
+enable-server
+etcd-config
+etcd_mutation_timeout
+etcd-prefix
+etcd-server
+etcd-servers
+event-ttl
+executor-bindall
+executor-cgroup-prefix
+executor-logv
+executor-path
+executor-suicide-timeout
+experimental-prefix
+external-hostname
+failover-timeout
+file-check-frequency
+file_content
+file_mode
+file_perm
+file-suffix
+forward-services
+framework-name
+framework-weburi
+fs_type
+func-dest
+fuzz-iters
+gce-project
+gce-zone
+gke-cluster
+gke_context
+google-json-key
+grace-period
+grace-period
+grace-period
+ha-domain
+healthz-bind-address
+healthz-port
+hostname-override
+host-network-sources
+host_port_endpoints
+http-check-frequency
+http-port
+ignore-not-found
+ignore-not-found
+image-gc-high-threshold
+image-gc-low-threshold
+insecure-bind-address
+insecure-port
+insecure-skip-tls-verify
+jenkins-host
+jenkins-jobs
+km-path
+kubecfg_file
+kubectl-path
+kubelet-cadvisor-port
+kubelet-certificate-authority
+kubelet-client-certificate
+kubelet-client-key
+kubelet-docker-endpoint
+kubelet-host-network-sources
+kubelet-https
+kubelet-network-plugin
+kubelet-pod-infra-container-image
+kubelet-port
+kubelet-root-dir
+kubelet-sync-frequency
+kubelet-timeout
+kube-master
+kube_master_url
+label-columns
+last-release-pr
+legacy-userspace-proxy
+log_flush_frequency
+long-running-request-regexp
+low-diskspace-threshold-mb
+manifest-url
+manifest-url-header
+master-service-namespace
+max-concurrency
+max-connection-bytes-per-sec
+maximum-dead-containers
+maximum-dead-containers-per-container
+max_in_flight
+max-log-age
+max-log-backups
+max-log-size
+max-outgoing-burst
+max-outgoing-qps
+max_par
+max-pods
+max-requests-inflight
+mesos-authentication-principal
+mesos-authentication-provider
+mesos-authentication-secret-file
+mesos-master
+mesos-role
+mesos-user
+minimum-container-ttl-duration
+minion-max-log-age
+minion-max-log-backups
+minion-max-log-size
+min-pr-number
+min-request-timeout
+namespace-sync-period
+network-plugin
+network-plugin-dir
+new_file_0644
+new_file_0666
+new_file_0777
+node-instance-group
+node-monitor-grace-period
+node-monitor-period
+node-startup-grace-period
+node-status-update-frequency
+node-sync-period
+no-headers
+num-nodes
+oom-score-adj
+output-version
+out-version
+pod-cidr
+pod-eviction-timeout
+pod-infra-container-image
+pods_per_node
+pods_per_node
+policy-config-file
+poll-interval
+portal-net
+private-mountns
+prom-push-gateway
+proxy-bindall
+proxy-logv
+proxy-port-range
+public-address-override
+public-ip
+pvclaimbinder-sync-period
+read-only-port
+really-crash-for-testing
+reconcile-cooldown
+reconcile-interval
+register-node
+register-retry-count
+registry-burst
+registry-qps
+reject-methods
+reject-paths
+repo-root
+report-dir
+required-contexts
+resource-container
+resource-quota-sync-period
+resource-version
+root-ca-file
+root-dir
+run-proxy
+runtime-config
+scheduler-config
+secure-port
+service-account-key-file
+service-account-lookup
+service-account-private-key-file
+service-address
+service-cluster-ip-range
+service-node-port-range
+service-node-ports
+service-sync-period
+session-affinity
+shutdown-fd
+shutdown-fifo
+skip-munges
+source-file
+ssh-keyfile
+ssh-user
+static-pods-config
+stats-port
+storage-version
+streaming-connection-idle-timeout
+suicide-timeout
+sync-frequency
+system-container
+target-port
+tcp-services
+test_args
+tls-cert-file
+tls-private-key-file
+token-auth-file
+ttl-secs
+unix-socket
+update-period
+upgrade-target
+up_to
+up_to
+use-kubernetes-cluster-service
+user-whitelist
+valid_flag
+watch-only
+whitelist-override-label
+www-prefix

hooks/pre-commit

@@ -53,6 +53,22 @@ else
 fi
 echo "${reset}"
 
+allfiles=($(git diff --cached --name-only --diff-filter ACM | grep -v -e "third_party" -e "Godeps"))
+
+echo -ne "Checking for problems with flag names... "
+invalid_flag_lines=$(hack/verify-flags-underscore.py "${allfiles[@]}")
+if [[ "${invalid_flag_lines:-}" != "" ]]; then
+  echo "${red}ERROR!"
+  echo "There appear to be problems with the following"
+  for line in "${invalid_flag_lines[@]}"; do
+    echo "  ${line}"
+  done
+  exit_code=1
+else
+  echo "${green}OK"
+fi
+echo "${reset}"
+
 echo -ne "Checking for API descriptions... "
 files_need_description=()
 # Check API schema definitions for field descriptions

shippable.yml

@@ -32,6 +32,7 @@ install:
   - ./hack/verify-gofmt.sh
   - ./hack/verify-boilerplate.sh
   - ./hack/verify-description.sh
+  - ./hack/verify-flags-underscore.py
   - ./hack/travis/install-std-race.sh
   - ./hack/verify-generated-conversions.sh
   - ./hack/verify-generated-deep-copies.sh