*: reflow comments to 80 characters
This reformats the entire Metropolis codebase to have comments no longer
than 80 characters, implementing CR/66.
This has been done half manually, as we don't have a good integration
between commentwrap/Bazel, but that can be implemented if we decide to
go for this tool/limit.
Change-Id: If1fff0b093ef806f5dc00551c11506e8290379d0
diff --git a/metropolis/node/core/consensus/ca/ca.go b/metropolis/node/core/consensus/ca/ca.go
index 6bcc19b..e0c56b0 100644
--- a/metropolis/node/core/consensus/ca/ca.go
+++ b/metropolis/node/core/consensus/ca/ca.go
@@ -73,19 +73,21 @@
CACert *x509.Certificate
CACertRaw []byte
- // bootstrapIssued are certificates that have been issued by the CA before it has been successfully Saved to etcd.
+ // bootstrapIssued are certificates that have been issued by the CA before
+ // it has been successfully Saved to etcd.
bootstrapIssued [][]byte
- // canBootstrapIssue is set on CAs that have been created by New and not yet stored to etcd. If not set,
- // certificates cannot be issued in-memory.
+ // canBootstrapIssue is set on CAs that have been created by New and not
+ // yet stored to etcd. If not set, certificates cannot be issued in-memory.
canBootstrapIssue bool
}
-// Workaround for https://github.com/golang/go/issues/26676 in Go's crypto/x509. Specifically Go
-// violates Section 4.2.1.2 of RFC 5280 without this.
+// Workaround for https://github.com/golang/go/issues/26676 in Go's
+// crypto/x509. Specifically Go violates Section 4.2.1.2 of RFC 5280 without
+// this.
// Fixed for 1.15 in https://go-review.googlesource.com/c/go/+/227098/.
//
-// Taken from https://github.com/FiloSottile/mkcert/blob/master/cert.go#L295 written by one of Go's
-// crypto engineers (BSD 3-clause).
+// Taken from https://github.com/FiloSottile/mkcert/blob/master/cert.go#L295
+// written by one of Go's crypto engineers (BSD 3-clause).
func calculateSKID(pubKey crypto.PublicKey) ([]byte, error) {
spkiASN1, err := x509.MarshalPKIXPublicKey(pubKey)
if err != nil {
@@ -104,8 +106,9 @@
return skid[:], nil
}
-// New creates a new certificate authority with the given common name. The newly created CA will be stored in memory
-// until committed to etcd by calling .Save.
+// New creates a new certificate authority with the given common name. The
+// newly created CA will be stored in memory until committed to etcd by calling
+// .Save.
func New(name string) (*CA, error) {
pubKey, privKey, err := ed25519.GenerateKey(rand.Reader)
if err != nil {
@@ -159,7 +162,8 @@
resp, err := kv.Txn(ctx).Then(
clientv3.OpGet(pathCACertificate),
clientv3.OpGet(pathCAKey),
- // We only read the CRL to ensure it exists on etcd (and early fail on inconsistency)
+ // We only read the CRL to ensure it exists on etcd (and early fail on
+ // inconsistency)
clientv3.OpGet(pathCACRL)).Commit()
if err != nil {
return nil, fmt.Errorf("failed to retrieve CA from etcd: %w", err)
@@ -198,7 +202,8 @@
}, nil
}
-// Save stores a newly created CA into etcd, committing both the CA data and any certificates issued until then.
+// Save stores a newly created CA into etcd, committing both the CA data and
+// any certificates issued until then.
func (c *CA) Save(ctx context.Context, kv clientv3.KV) error {
crl, err := c.makeCRL(nil)
if err != nil {
@@ -233,8 +238,9 @@
return nil
}
-// Issue issues a certificate. If kv is non-nil, the newly issued certificate will be immediately stored to etcd,
-// otherwise it will be kept in memory (until .Save is called). Certificates can only be issued to memory on
+// Issue issues a certificate. If kv is non-nil, the newly issued certificate
+// will be immediately stored to etcd, otherwise it will be kept in memory
+// (until .Save is called). Certificates can only be issued to memory on
// newly-created CAs that have not been saved to etcd yet.
func (c *CA) Issue(ctx context.Context, kv clientv3.KV, commonName string) (cert []byte, privkey []byte, err error) {
serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 127)
@@ -297,9 +303,10 @@
return crl, nil
}
-// Revoke revokes a certificate by hostname. The selected hostname will be added to the CRL stored in etcd. This call
-// might fail (safely) if a simultaneous revoke happened that caused the CRL to be bumped. The call can be then retried
-// safely.
+// Revoke revokes a certificate by hostname. The selected hostname will be
+// added to the CRL stored in etcd. This call might fail (safely) if a
+// simultaneous revoke happened that caused the CRL to be bumped. The call can
+// be then retried safely.
func (c *CA) Revoke(ctx context.Context, kv clientv3.KV, hostname string) error {
res, err := kv.Txn(ctx).Then(
clientv3.OpGet(pathCACRL),
@@ -383,8 +390,9 @@
return nil
}
-// WaitCRLChange returns a channel that will receive a CRLUpdate any time the remote CRL changed. Immediately after
-// calling this method, the current CRL is retrieved from the cluster and put into the channel.
+// WaitCRLChange returns a channel that will receive a CRLUpdate any time the
+// remote CRL changed. Immediately after calling this method, the current CRL
+// is retrieved from the cluster and put into the channel.
func (c *CA) WaitCRLChange(ctx context.Context, kv clientv3.KV, w clientv3.Watcher) <-chan CRLUpdate {
C := make(chan CRLUpdate)
@@ -424,16 +432,20 @@
return C
}
-// CRLUpdate is emitted for every remote CRL change, and spuriously on ever new WaitCRLChange.
+// CRLUpdate is emitted for every remote CRL change, and spuriously on ever new
+// WaitCRLChange.
type CRLUpdate struct {
- // The new (or existing, in the case of the first call) CRL. If nil, Err will be set.
+ // The new (or existing, in the case of the first call) CRL. If nil, Err
+ // will be set.
CRL []byte
- // If set, an error occurred and the WaitCRLChange call must be restarted. If set, CRL will be nil.
+ // If set, an error occurred and the WaitCRLChange call must be restarted.
+ // If set, CRL will be nil.
Err error
}
-// GetCurrentCRL returns the current CRL for the CA. This should only be used for one-shot operations like
-// bootstrapping a new node that doesn't yet have access to etcd - otherwise, WaitCRLChange shoulde be used.
+// GetCurrentCRL returns the current CRL for the CA. This should only be used
+// for one-shot operations like bootstrapping a new node that doesn't yet have
+// access to etcd - otherwise, WaitCRLChange shoulde be used.
func (c *CA) GetCurrentCRL(ctx context.Context, kv clientv3.KV) ([]byte, error) {
initial, err := kv.Get(ctx, pathCACRL)
if err != nil {
diff --git a/metropolis/node/core/consensus/consensus.go b/metropolis/node/core/consensus/consensus.go
index 683db19..ed44140 100644
--- a/metropolis/node/core/consensus/consensus.go
+++ b/metropolis/node/core/consensus/consensus.go
@@ -14,19 +14,22 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-// Package consensus implements a managed etcd cluster member service, with a self-hosted CA system for issuing peer
-// certificates. Currently each Metropolis node runs an etcd member, and connects to the etcd member locally over a
-// domain socket.
+// Package consensus implements a managed etcd cluster member service, with a
+// self-hosted CA system for issuing peer certificates. Currently each
+// Metropolis node runs an etcd member, and connects to the etcd member locally
+// over a domain socket.
//
// The service supports two modes of startup:
-// - initializing a new cluster, by bootstrapping the CA in memory, starting a cluster, committing the CA to etcd
-// afterwards, and saving the new node's certificate to local storage
-// - joining an existing cluster, using certificates from local storage and loading the CA from etcd. This flow is also
-// used when the node joins a cluster for the first time (then the certificates required must be provisioned
-// externally before starting the consensus service).
+// - initializing a new cluster, by bootstrapping the CA in memory, starting a
+// cluster, committing the CA to etcd afterwards, and saving the new node's
+// certificate to local storage
+// - joining an existing cluster, using certificates from local storage and
+// loading the CA from etcd. This flow is also used when the node joins a
+// cluster for the first time (then the certificates required must be
+// provisioned externally before starting the consensus service).
//
-// Regardless of how the etcd member service was started, the resulting running service is further managed and used
-// in the same way.
+// Regardless of how the etcd member service was started, the resulting running
+// service is further managed and used in the same way.
//
package consensus
@@ -59,8 +62,9 @@
// The configuration with which the service was started. This is immutable.
config *Config
- // stateMu guards state. This is locked internally on public methods of Service that require access to state. The
- // state might be recreated on service restart.
+ // stateMu guards state. This is locked internally on public methods of
+ // Service that require access to state. The state might be recreated on
+ // service restart.
stateMu sync.Mutex
state *state
}
@@ -71,8 +75,8 @@
ready atomic.Bool
ca *ca.CA
- // cl is an etcd client that loops back to the localy running etcd server. This runs over the Client unix domain
- // socket that etcd starts.
+ // cl is an etcd client that loops back to the localy running etcd server.
+ // This runs over the Client unix domain socket that etcd starts.
cl *clientv3.Client
}
@@ -82,16 +86,19 @@
// Ephemeral directory for etcd.
Ephemeral *localstorage.EphemeralConsensusDirectory
- // Name is the cluster name. This must be the same amongst all etcd members within one cluster.
+ // Name is the cluster name. This must be the same amongst all etcd members
+ // within one cluster.
Name string
- // NewCluster selects whether the etcd member will start a new cluster and bootstrap a CA and the first member
- // certificate, or load existing PKI certificates from disk.
+ // NewCluster selects whether the etcd member will start a new cluster and
+ // bootstrap a CA and the first member certificate, or load existing PKI
+ // certificates from disk.
NewCluster bool
- // Port is the port at which this cluster member will listen for other members. If zero, defaults to the global
- // Metropolis setting.
+ // Port is the port at which this cluster member will listen for other
+ // members. If zero, defaults to the global Metropolis setting.
Port int
- // externalHost is used by tests to override the address at which etcd should listen for peer connections.
+ // externalHost is used by tests to override the address at which etcd
+ // should listen for peer connections.
externalHost string
}
@@ -101,8 +108,8 @@
}
}
-// configure transforms the service configuration into an embedded etcd configuration. This is pure and side effect
-// free.
+// configure transforms the service configuration into an embedded etcd
+// configuration. This is pure and side effect free.
func (s *Service) configure(ctx context.Context) (*embed.Config, error) {
if err := s.config.Ephemeral.MkdirAll(0700); err != nil {
return nil, fmt.Errorf("failed to create ephemeral directory: %w", err)
@@ -166,8 +173,8 @@
return cfg, nil
}
-// Run is a Supervisor runnable that starts the etcd member service. It will become healthy once the member joins the
-// cluster successfully.
+// Run is a Supervisor runnable that starts the etcd member service. It will
+// become healthy once the member joins the cluster successfully.
func (s *Service) Run(ctx context.Context) error {
st := &state{
ready: *atomic.NewBool(false),
@@ -298,8 +305,8 @@
return ctx.Err()
}
-// watchCRL is a sub-runnable of the etcd cluster member service that updates the on-local-storage CRL to match the
-// newest available version in etcd.
+// watchCRL is a sub-runnable of the etcd cluster member service that updates
+// the on-local-storage CRL to match the newest available version in etcd.
func (s *Service) watchCRL(ctx context.Context) error {
s.stateMu.Lock()
cl := s.state.cl
@@ -339,9 +346,10 @@
continue
}
- // We always call PromoteMember since the metadata necessary to decide if we should is private.
- // Luckily etcd already does sanity checks internally and will refuse to promote nodes that aren't
- // connected or are still behind on transactions.
+ // We always call PromoteMember since the metadata necessary to
+ // decide if we should is private. Luckily etcd already does
+ // sanity checks internally and will refuse to promote nodes that
+ // aren't connected or are still behind on transactions.
if _, err := st.etcd.Server.PromoteMember(ctx, uint64(member.ID)); err != nil {
supervisor.Logger(ctx).Infof("Failed to promote consensus node %s: %v", member.Name, err)
} else {
@@ -371,7 +379,8 @@
}
func (s *Service) WaitReady(ctx context.Context) error {
- // TODO(q3k): reimplement the atomic ready flag as an event synchronization mechanism
+ // TODO(q3k): reimplement the atomic ready flag as an event synchronization
+ // mechanism
if s.IsReady() {
return nil
}
@@ -411,8 +420,9 @@
return s.state.cl.Cluster
}
-// MemberInfo returns information about this etcd cluster member: its ID and name. This will block until this
-// information is available (ie. the cluster status is Ready).
+// MemberInfo returns information about this etcd cluster member: its ID and
+// name. This will block until this information is available (ie. the cluster
+// status is Ready).
func (s *Service) MemberInfo(ctx context.Context) (id uint64, name string, err error) {
if err = s.WaitReady(ctx); err != nil {
err = fmt.Errorf("when waiting for cluster readiness: %w", err)