Skip to content

Commit

Permalink
remove deprecated constellation create flags (#2325)
Browse files Browse the repository at this point in the history
* chore: clean-up TODOs

* cli: make OpenStack error explicit

* cli: remove deprecated flags

* config: require DeployCSIDriver field
  • Loading branch information
3u13r committed Sep 8, 2023
1 parent 5960025 commit a25c90e
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 134 deletions.
2 changes: 0 additions & 2 deletions 3rdparty/bazel/org_openssl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ cc_test(
],
"//conditions:default": [],
}),
# TODO support OpenSSL on Mac
target_compatible_with = [
"@platforms//os:linux",
],
Expand All @@ -27,7 +26,6 @@ cc_test(

build_test(
name = "build_test",
# TODO support OpenSSL on Mac
target_compatible_with = [
"@platforms//os:linux",
],
Expand Down
3 changes: 1 addition & 2 deletions cli/internal/cloudcmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,8 @@ func normalizeAzureURIs(vars *terraform.AzureClusterVariables) *terraform.AzureC
}

func (c *Creator) createOpenStack(ctx context.Context, cl tfResourceClient, opts CreateOptions) (tfOutput terraform.ApplyOutput, retErr error) {
// TODO(malt3): Remove this once OpenStack is supported.
if os.Getenv("CONSTELLATION_OPENSTACK_DEV") != "1" {
return terraform.ApplyOutput{}, errors.New("OpenStack isn't supported yet")
return terraform.ApplyOutput{}, errors.New("Constellation must be fine-tuned to your OpenStack deployment. Please create an issue or contact Edgeless Systems at https://edgeless.systems/contact/")
}
if _, hasOSAuthURL := os.LookupEnv("OS_AUTH_URL"); !hasOSAuthURL && opts.Config.Provider.OpenStack.Cloud == "" {
return terraform.ApplyOutput{}, errors.New(
Expand Down
1 change: 0 additions & 1 deletion cli/internal/cloudcmd/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
)

func TestCreator(t *testing.T) {
// TODO(malt3): remove once OpenStack is fully supported.
t.Setenv("CONSTELLATION_OPENSTACK_DEV", "1")
failOnNonAMD64 := (runtime.GOARCH != "amd64") || (runtime.GOOS != "linux")
ip := "192.0.2.1"
Expand Down
5 changes: 0 additions & 5 deletions cli/internal/cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ func NewCreateCmd() *cobra.Command {
RunE: runCreate,
}
cmd.Flags().BoolP("yes", "y", false, "create the cluster without further confirmation")
// TODO(malt3): remove deprecated flags in v2.11+
cmd.Flags().IntP("control-plane-nodes", "c", 0, "number of control-plane nodes")
cmd.Flags().IntP("worker-nodes", "w", 0, "number of worker nodes")
must(cmd.Flags().MarkDeprecated("control-plane-nodes", "configure the number of control-plane nodes in the configuration file"))
must(cmd.Flags().MarkDeprecated("worker-nodes", "configure the number of worker nodes in the configuration file"))
return cmd
}

Expand Down
139 changes: 40 additions & 99 deletions cli/internal/cmd/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ package cmd
import (
"bytes"
"errors"
"strconv"
"testing"

"github.com/edgelesssys/constellation/v2/cli/internal/clusterid"
Expand Down Expand Up @@ -46,68 +45,30 @@ func TestCreate(t *testing.T) {
wantAbort bool
}{
"create": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{id: idFile},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(2),
yesFlag: true,
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{id: idFile},
provider: cloudprovider.GCP,
yesFlag: true,
},
"interactive": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{id: idFile},
provider: cloudprovider.Azure,
controllerCountFlag: intPtr(2),
workerCountFlag: intPtr(1),
stdin: "yes\n",
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{id: idFile},
provider: cloudprovider.Azure,
stdin: "yes\n",
},
"interactive abort": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(1),
stdin: "no\n",
wantAbort: true,
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
stdin: "no\n",
wantAbort: true,
},
"interactive error": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(1),
stdin: "foo\nfoo\nfoo\n",
wantErr: true,
},
"flag control-plane-count invalid": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(0),
workerCountFlag: intPtr(3),
wantErr: true,
},
"flag worker-count invalid": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(3),
workerCountFlag: intPtr(0),
wantErr: true,
},
"flag control-plane-count missing": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
workerCountFlag: intPtr(3),
wantErr: true,
},
"flag worker-count missing": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(3),
wantErr: true,
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
stdin: "foo\nfoo\nfoo\n",
wantErr: true,
},
"old adminConf in directory": {
setupFs: func(require *require.Assertions, csp cloudprovider.Provider) afero.Fs {
Expand All @@ -117,12 +78,10 @@ func TestCreate(t *testing.T) {
require.NoError(fileHandler.WriteYAML(constants.ConfigFilename, defaultConfigWithExpectedMeasurements(t, config.Default(), csp)))
return fs
},
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(1),
yesFlag: true,
wantErr: true,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
yesFlag: true,
wantErr: true,
},
"old masterSecret in directory": {
setupFs: func(require *require.Assertions, csp cloudprovider.Provider) afero.Fs {
Expand All @@ -132,30 +91,24 @@ func TestCreate(t *testing.T) {
require.NoError(fileHandler.WriteYAML(constants.ConfigFilename, defaultConfigWithExpectedMeasurements(t, config.Default(), csp)))
return fs
},
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(1),
yesFlag: true,
wantErr: true,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
yesFlag: true,
wantErr: true,
},
"config does not exist": {
setupFs: func(a *require.Assertions, p cloudprovider.Provider) afero.Fs { return afero.NewMemMapFs() },
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(1),
yesFlag: true,
wantErr: true,
setupFs: func(a *require.Assertions, p cloudprovider.Provider) afero.Fs { return afero.NewMemMapFs() },
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
yesFlag: true,
wantErr: true,
},
"create error": {
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{createErr: someErr},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(1),
yesFlag: true,
wantErr: true,
setupFs: fsWithDefaultConfig,
creator: &stubCloudCreator{createErr: someErr},
provider: cloudprovider.GCP,
yesFlag: true,
wantErr: true,
},
"write id file error": {
setupFs: func(require *require.Assertions, csp cloudprovider.Provider) afero.Fs {
Expand All @@ -164,12 +117,10 @@ func TestCreate(t *testing.T) {
require.NoError(fileHandler.WriteYAML(constants.ConfigFilename, defaultConfigWithExpectedMeasurements(t, config.Default(), csp)))
return afero.NewReadOnlyFs(fs)
},
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
controllerCountFlag: intPtr(1),
workerCountFlag: intPtr(1),
yesFlag: true,
wantErr: true,
creator: &stubCloudCreator{},
provider: cloudprovider.GCP,
yesFlag: true,
wantErr: true,
},
}

Expand All @@ -189,12 +140,6 @@ func TestCreate(t *testing.T) {
if tc.yesFlag {
require.NoError(cmd.Flags().Set("yes", "true"))
}
if tc.controllerCountFlag != nil {
require.NoError(cmd.Flags().Set("control-plane-nodes", strconv.Itoa(*tc.controllerCountFlag)))
}
if tc.workerCountFlag != nil {
require.NoError(cmd.Flags().Set("worker-nodes", strconv.Itoa(*tc.workerCountFlag)))
}

fileHandler := file.NewHandler(tc.setupFs(require, tc.provider))
c := &createCmd{log: logger.NewTest(t)}
Expand Down Expand Up @@ -332,7 +277,3 @@ func TestValidateCLIandConstellationVersionCompatibility(t *testing.T) {
})
}
}

func intPtr(i int) *int {
return &i
}
2 changes: 1 addition & 1 deletion cli/internal/cmd/upgradeapply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (u *stubKubernetesUpgrader) ExtendClusterConfigCertSANs(_ context.Context,
return nil
}

// TODO(v2.11): Remove this function.
// TODO(v2.11): Remove this function after v2.11 is released.
func (u *stubKubernetesUpgrader) RemoveAttestationConfigHelmManagement(_ context.Context) error {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/kubecmd/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (k *KubeCmd) BackupCRs(ctx context.Context, crds []apiextensionsv1.CustomRe
k.log.Debugf("Creating backup for resource type: %s", crd.Name)

// Iterate over all versions of the CRD
// TODO: Consider iterating over crd.Status.StoredVersions instead
// TODO(daniel-weisse): Consider iterating over crd.Status.StoredVersions instead
// Currently, we have to ignore not-found errors, because a CRD might define
// a version that is not installed in the cluster.
// With the StoredVersions field, we could only iterate over the installed versions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ resource "aws_launch_template" "launch_template" {
# Disable SMT. We are already disabling it inside the image.
# Disabling SMT only in the image, not in the Hypervisor creates problems.
# Thus, also disable it in the Hypervisor.
# TODO (derpsteb): reenable once AWS confirms it's safe to do so.
# TODO(derpsteb): reenable once AWS confirms it's safe to do so.
# threads_per_core = 1
# When setting threads_per_core we also have to set core_count.
# For the currently supported SNP instance families (C6a, M6a, R6a) default_cores
Expand Down
12 changes: 6 additions & 6 deletions cli/internal/terraform/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Variables interface {
// ClusterVariables should be used in places where a cluster is created.
type ClusterVariables interface {
Variables
// TODO (derpsteb): Rename this function once we have introduced an interface for config.Config.
// TODO(derpsteb): Rename this function once we have introduced an interface for config.Config.
// GetCreateMAA does not follow Go's naming convention because we need to keep the CreateMAA property public for now.
// There are functions creating Variables objects outside of this package.
// These functions can only be moved into this package once we have introduced an interface for config.Config,
Expand Down Expand Up @@ -69,7 +69,7 @@ type AWSClusterVariables struct {
}

// GetCreateMAA gets the CreateMAA variable.
// TODO (derpsteb): Rename this function once we have introduced an interface for config.Config.
// TODO(derpsteb): Rename this function once we have introduced an interface for config.Config.
func (a *AWSClusterVariables) GetCreateMAA() bool {
return false
}
Expand Down Expand Up @@ -134,7 +134,7 @@ type GCPClusterVariables struct {
}

// GetCreateMAA gets the CreateMAA variable.
// TODO (derpsteb): Rename this function once we have introduced an interface for config.Config.
// TODO(derpsteb): Rename this function once we have introduced an interface for config.Config.
func (g *GCPClusterVariables) GetCreateMAA() bool {
return false
}
Expand Down Expand Up @@ -206,7 +206,7 @@ type AzureClusterVariables struct {
}

// GetCreateMAA gets the CreateMAA variable.
// TODO (derpsteb): Rename this function once we have introduced an interface for config.Config.
// TODO(derpsteb): Rename this function once we have introduced an interface for config.Config.
func (a *AzureClusterVariables) GetCreateMAA() bool {
if a.CreateMAA == nil {
return false
Expand Down Expand Up @@ -278,7 +278,7 @@ type OpenStackClusterVariables struct {
}

// GetCreateMAA gets the CreateMAA variable.
// TODO (derpsteb): Rename this function once we have introduced an interface for config.Config.
// TODO(derpsteb): Rename this function once we have introduced an interface for config.Config.
func (o *OpenStackClusterVariables) GetCreateMAA() bool {
return false
}
Expand Down Expand Up @@ -349,7 +349,7 @@ type QEMUVariables struct {
}

// GetCreateMAA gets the CreateMAA variable.
// TODO (derpsteb): Rename this function once we have introduced an interface for config.Config.
// TODO(derpsteb): Rename this function once we have introduced an interface for config.Config.
func (q *QEMUVariables) GetCreateMAA() bool {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion dev-docs/miniconstellation/setup-miniconstellation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ curl -LO https://github.com/edgelesssys/constellation/releases/latest/download/c
sudo install constellation-linux-amd64 /usr/local/bin/constellation

# Start docker service and auto start on boot
# TODO should be done in cloud-init but was not done in my test case
# TODO(elchead) should be done in cloud-init but was not done in my test case
sudo systemctl start docker.service && sudo systemctl enable docker.service
echo "Waiting for docker service to be active..."
# Wait at most 20min
Expand Down
2 changes: 1 addition & 1 deletion internal/api/attestationconfigapi/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func newFetcherWithClientAndVerifier(client apifetcher.HTTPClient, cosignVerifie

// FetchAzureSEVSNPVersionList fetches the version list information from the config API.
func (f *fetcher) FetchAzureSEVSNPVersionList(ctx context.Context, attestation AzureSEVSNPVersionList) (AzureSEVSNPVersionList, error) {
// TODO (derpsteb): Replace with FetchAndVerify once we move to v2 of the config API.
// TODO(derpsteb): Replace with FetchAndVerify once we move to v2 of the config API.
return apifetcher.Fetch(ctx, f.HTTPClient, attestation)
}

Expand Down
14 changes: 2 additions & 12 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ type AWSConfig struct {
IAMProfileWorkerNodes string `yaml:"iamProfileWorkerNodes" validate:"required"`
// description: |
// Deploy Persistent Disk CSI driver with on-node encryption. For details see: https://docs.edgeless.systems/constellation/architecture/encrypted-storage
DeployCSIDriver *bool `yaml:"deployCSIDriver"` // TODO (msanft): after v2.9 release re-enable "required" validation
DeployCSIDriver *bool `yaml:"deployCSIDriver" validate:"required"`
}

// AzureConfig are Azure specific configuration values used by the CLI.
Expand Down Expand Up @@ -472,16 +472,6 @@ func New(fileHandler file.Handler, name string, fetcher attestationconfigapi.Fet
c.Provider.OpenStack.Password = openstackPassword
}

// Backwards compatibility: configs without the field `deployCSIDriver` are valid in version 2.8.
// TODO (msanft): v2.9. Remove after v2.9 release.
if c.Provider.AWS != nil && c.Provider.AWS.DeployCSIDriver == nil {
fmt.Fprintln(
os.Stderr,
"WARNING: 'provider.aws.deployCSIDriver' not set. The key will be required in v2.10. Defaulting to 'false'.",
)
c.Provider.AWS.DeployCSIDriver = toPtr(false)
}

return c, c.Validate(force)
}

Expand Down Expand Up @@ -949,7 +939,7 @@ func (c AWSSEVSNP) EqualTo(other AttestationCfg) (bool, error) {
if !ok {
return false, fmt.Errorf("cannot compare %T with %T", c, other)
}
// TODO (derpsteb): reenable launchMeasurement once SNP is fixed on AWS.
// TODO(derpsteb): reenable launchMeasurement once SNP is fixed on AWS.
// if !bytes.Equal(c.LaunchMeasurement.Expected, otherCfg.LaunchMeasurement.Expected) {
// return false, nil
// }
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config_doc.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/config/migration/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ type AWSSEVSNP struct {
// description: |
// Expected TPM measurements.
Measurements measurements.M `json:"measurements" yaml:"measurements" validate:"required,no_placeholders"`
// TODO (derpsteb): reenable launchMeasurement once SNP is fixed on AWS.
// TODO(derpsteb): reenable launchMeasurement once SNP is fixed on AWS.
// description: |
// Expected launch measurement in SNP report.
// LaunchMeasurement measurements.Measurement `json:"launchMeasurement" yaml:"launchMeasurement" validate:"required"`
Expand Down

0 comments on commit a25c90e

Please sign in to comment.