Improve documentation, remove dead code plus some minor refactorings
This improves our code-to-comments ratio by a lot.
On the refactorings:
- Simplify the cluster join mode to just a single protobuf message -
a node can either join an existing cluster or bootstrap a new one.
All of the node-level setup like hostname and trust backend is done
using the setup call, since those are identical for both cases.
- We don't need a node name separate from the hostname. Ideally, we would
get rid of IP addresses for etcd as well.
- Google API design guidelines suggest the `List` term (vs. `Get`).
- Add username to comments for consistency. I think the names provide
useful context, but git blame is a thing. What do you think?
- Fixed or silenced some ignored error checks in preparation of using
an errcheck linter. Especially during early boot, many errors are
obviously not recoverable, but logging them can provide useful debugging info.
- Split up the common package into smaller subpackages.
- Remove the audit package (this will be a separate service that probably
uses it own database, rather than etcd).
- Move storage constants to storage package.
- Remove the unused KV type.
I also added a bunch of TODO comments with discussion points.
Added both of you as blocking reviewers - please comment if I
misunderstood any of your code.
Test Plan: Everything compiles and scripts:launch works (for whatever that's worth).
X-Origin-Diff: phab/D235
GitOrigin-RevId: 922fec5076e8d683e1138f26d2cb490de64a9777
diff --git a/core/internal/common/BUILD.bazel b/core/internal/common/BUILD.bazel
index 4312b88..1adbfd8 100644
--- a/core/internal/common/BUILD.bazel
+++ b/core/internal/common/BUILD.bazel
@@ -2,19 +2,8 @@
go_library(
name = "go_default_library",
- srcs = [
- "grpc.go",
- "service.go",
- "setup.go",
- "storage.go",
- "util.go",
- ],
+ srcs = ["setup.go"],
importpath = "git.monogon.dev/source/nexantic.git/core/internal/common",
- visibility = ["//core:__subpackages__"],
- deps = [
- "//core/api/api:go_default_library",
- "//core/api/common:go_default_library",
- "@org_golang_google_grpc//:go_default_library",
- "@org_uber_go_zap//:go_default_library",
- ],
+ visibility = ["//:__subpackages__"],
+ deps = ["//core/api/api:go_default_library"],
)
diff --git a/core/internal/common/grpc/BUILD.bazel b/core/internal/common/grpc/BUILD.bazel
new file mode 100644
index 0000000..85661c6
--- /dev/null
+++ b/core/internal/common/grpc/BUILD.bazel
@@ -0,0 +1,12 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+ name = "go_default_library",
+ srcs = ["grpc.go"],
+ importpath = "git.monogon.dev/source/nexantic.git/core/internal/common/grpc",
+ visibility = ["//:__subpackages__"],
+ deps = [
+ "//core/api/api:go_default_library",
+ "@org_golang_google_grpc//:go_default_library",
+ ],
+)
diff --git a/core/internal/common/grpc.go b/core/internal/common/grpc/grpc.go
similarity index 98%
rename from core/internal/common/grpc.go
rename to core/internal/common/grpc/grpc.go
index 5512a5c..e9cfed0 100644
--- a/core/internal/common/grpc.go
+++ b/core/internal/common/grpc/grpc.go
@@ -14,7 +14,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package common
+package grpc
import (
"git.monogon.dev/source/nexantic.git/core/generated/api"
diff --git a/core/internal/common/service/BUILD.bazel b/core/internal/common/service/BUILD.bazel
new file mode 100644
index 0000000..a06ca83
--- /dev/null
+++ b/core/internal/common/service/BUILD.bazel
@@ -0,0 +1,9 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+ name = "go_default_library",
+ srcs = ["service.go"],
+ importpath = "git.monogon.dev/source/nexantic.git/core/internal/common/service",
+ visibility = ["//:__subpackages__"],
+ deps = ["@org_uber_go_zap//:go_default_library"],
+)
diff --git a/core/internal/common/service.go b/core/internal/common/service/service.go
similarity index 96%
rename from core/internal/common/service.go
rename to core/internal/common/service/service.go
index 3bdc1f9..e093ff6 100644
--- a/core/internal/common/service.go
+++ b/core/internal/common/service/service.go
@@ -14,7 +14,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package common
+package service
import (
"errors"
@@ -74,7 +74,7 @@
return nil
}
-// Stop stops the service. THis is an atomic operation and should only be called on a running service.
+// Stop stops the service. This is an atomic operation and should only be called on a running service.
func (b *BaseService) Stop() error {
b.mutex.Lock()
defer b.mutex.Unlock()
diff --git a/core/internal/common/setup.go b/core/internal/common/setup.go
index 331d29a..1124d27 100644
--- a/core/internal/common/setup.go
+++ b/core/internal/common/setup.go
@@ -18,20 +18,25 @@
import "git.monogon.dev/source/nexantic.git/core/generated/api"
+// TODO(leo): merge api and node packages and get rid of this extra layer of indirection?
+
type (
SetupService interface {
CurrentState() SmalltownState
GetJoinClusterToken() string
- SetupNewCluster(name string, externalHost string) error
+ SetupNewCluster() error
EnterJoinClusterMode() error
- JoinCluster(name string, clusterString string, externalHost string, certs *api.ConsensusCertificates) error
+ JoinCluster(initialCluster string, certs *api.ConsensusCertificates) error
}
SmalltownState string
)
const (
- StateSetupMode SmalltownState = "setup"
+ // Node is unprovisioned and waits for Setup to be called.
+ StateSetupMode SmalltownState = "setup"
+ // Setup() has been called, node waits for a JoinCluster or BootstrapCluster call.
StateClusterJoinMode SmalltownState = "join"
- StateConfigured SmalltownState = "configured"
+ // Node is fully provisioned.
+ StateConfigured SmalltownState = "configured"
)
diff --git a/core/internal/common/storage.go b/core/internal/common/storage.go
deleted file mode 100644
index caaa155..0000000
--- a/core/internal/common/storage.go
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright 2020 The Monogon Project Authors.
-//
-// SPDX-License-Identifier: Apache-2.0
-//
-// 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.
-
-package common
-
-import "errors"
-
-type DataPlace uint32
-
-const (
- PlaceESP DataPlace = 0
- PlaceData = 1
-)
-
-var (
- // ErrNotInitialized will be returned when trying to access a place that's not yet initialized
- ErrNotInitialized = errors.New("This place is not initialized")
- // ErrUnknownPlace will be returned when trying to access a place that's not known
- ErrUnknownPlace = errors.New("This place is not known")
-)
-
-type StorageManager interface {
- GetPathInPlace(place DataPlace, path string) (string, error)
-}
diff --git a/core/internal/common/util.go b/core/internal/common/util.go
deleted file mode 100644
index fc8a72b..0000000
--- a/core/internal/common/util.go
+++ /dev/null
@@ -1,33 +0,0 @@
-// Copyright 2020 The Monogon Project Authors.
-//
-// SPDX-License-Identifier: Apache-2.0
-//
-// 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.
-
-package common
-
-import "git.monogon.dev/source/nexantic.git/core/generated/common"
-
-func MapToKVs(input map[string]string) []*common.KV {
- kvs := make([]*common.KV, len(input))
-
- i := 0
- for key, item := range input {
- kvs[i] = &common.KV{
- Key: key,
- Value: []byte(item),
- }
- }
-
- return kvs
-}