Skip to content

Commit

Permalink
refactor: change isInternal variables to functions (zarf-dev#2768)
Browse files Browse the repository at this point in the history 
Signed-off-by: Austin Abro <AustinAbro321@gmail.com>
Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>
  • Loading branch information
@AustinAbro321 @chaospuppy
AustinAbro321 authored and chaospuppy committed Aug 5, 2024
1 parent d7d4690 commit e0a9df5
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 160 deletions.
2 changes: 1 addition & 1 deletion src/cmd/internal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ var createPackageRegistryToken = &cobra.Command{
}

// If we are setup to use an internal artifact server, create the artifact registry token
if state.ArtifactServer.InternalServer {
if state.ArtifactServer.IsInternal() {
tunnel, err := c.NewTunnel(cluster.ZarfNamespaceName, cluster.SvcResource, cluster.ZarfGitServerName, "", 0, cluster.ZarfGitServerPort)
if err != nil {
return err
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/tools/zarf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ var updateCredsCmd = &cobra.Command{
}

// Update artifact token (if internal)
if slices.Contains(args, message.ArtifactKey) && newState.ArtifactServer.PushToken == "" && newState.ArtifactServer.InternalServer {
if slices.Contains(args, message.ArtifactKey) && newState.ArtifactServer.PushToken == "" && newState.ArtifactServer.IsInternal() {
tunnel, err := c.NewTunnel(cluster.ZarfNamespaceName, cluster.SvcResource, cluster.ZarfGitServerName, "", 0, cluster.ZarfGitServerPort)
if err != nil {
return err
Expand Down Expand Up @@ -186,14 +186,14 @@ var updateCredsCmd = &cobra.Command{
// Update Zarf 'init' component Helm releases if present
h := helm.NewClusterOnly(&types.PackagerConfig{}, template.GetZarfVariableConfig(), newState, c)

if slices.Contains(args, message.RegistryKey) && newState.RegistryInfo.InternalRegistry {
if slices.Contains(args, message.RegistryKey) && newState.RegistryInfo.IsInternal() {
err = h.UpdateZarfRegistryValues(ctx)
if err != nil {
// Warn if we couldn't actually update the registry (it might not be installed and we should try to continue)
message.Warnf(lang.CmdToolsUpdateCredsUnableUpdateRegistry, err.Error())
}
}
if slices.Contains(args, message.GitKey) && newState.GitServer.InternalServer {
if slices.Contains(args, message.GitKey) && newState.GitServer.IsInternal() {
tunnel, err := c.NewTunnel(cluster.ZarfNamespaceName, cluster.SvcResource, cluster.ZarfGitServerName, "", 0, cluster.ZarfGitServerPort)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion src/internal/agent/hooks/flux-helmrepo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func mutateHelmRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluste

message.Debugf("original HelmRepo URL of (%s) got mutated to (%s)", src.Spec.URL, patchedURL)

patches := populateHelmRepoPatchOperations(patchedURL, zarfState.RegistryInfo.InternalRegistry)
patches := populateHelmRepoPatchOperations(patchedURL, zarfState.RegistryInfo.IsInternal())

patches = append(patches, getLabelPatch(src.Labels))

Expand Down
2 changes: 1 addition & 1 deletion src/internal/agent/hooks/flux-ocirepo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func mutateOCIRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster

message.Debugf("original OCIRepo URL of (%s) got mutated to (%s)", src.Spec.URL, patchedURL)

patches := populateOCIRepoPatchOperations(patchedURL, zarfState.RegistryInfo.InternalRegistry, patchedRef)
patches := populateOCIRepoPatchOperations(patchedURL, zarfState.RegistryInfo.IsInternal(), patchedRef)

patches = append(patches, getLabelPatch(src.Labels))
return &operations.Result{
Expand Down
2 changes: 1 addition & 1 deletion src/internal/packager/template/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func GetZarfTemplates(componentName string, state *types.ZarfState) (templateMap
// generateHtpasswd returns an htpasswd string for the current state's RegistryInfo.
func generateHtpasswd(regInfo *types.RegistryInfo) (string, error) {
// Only calculate this for internal registries to allow longer external passwords
if regInfo.InternalRegistry {
if regInfo.IsInternal() {
pushUser, err := utils.GetHtpasswdString(regInfo.PushUsername, regInfo.PushPassword)
if err != nil {
return "", fmt.Errorf("error generating htpasswd string: %w", err)
Expand Down
33 changes: 5 additions & 28 deletions src/pkg/cluster/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,21 +306,14 @@ func MergeZarfState(oldState *types.ZarfState, initOptions types.ZarfInitOptions
if slices.Contains(services, message.RegistryKey) {
// TODO: Replace use of reflections with explicit setting
newState.RegistryInfo = helpers.MergeNonZero(newState.RegistryInfo, initOptions.RegistryInfo)
// Set the state of the internal registry if it has changed
// TODO: Internal registry should be a function of the address and not a property.
if newState.RegistryInfo.Address == fmt.Sprintf("%s:%d", helpers.IPV4Localhost, newState.RegistryInfo.NodePort) {
newState.RegistryInfo.InternalRegistry = true
} else {
newState.RegistryInfo.InternalRegistry = false
}

// Set the new passwords if they should be autogenerated
if newState.RegistryInfo.PushPassword == oldState.RegistryInfo.PushPassword && oldState.RegistryInfo.InternalRegistry {
if newState.RegistryInfo.PushPassword == oldState.RegistryInfo.PushPassword && oldState.RegistryInfo.IsInternal() {
if newState.RegistryInfo.PushPassword, err = helpers.RandomString(types.ZarfGeneratedPasswordLen); err != nil {
return nil, fmt.Errorf("%s: %w", lang.ErrUnableToGenerateRandomSecret, err)
}
}
if newState.RegistryInfo.PullPassword == oldState.RegistryInfo.PullPassword && oldState.RegistryInfo.InternalRegistry {
if newState.RegistryInfo.PullPassword == oldState.RegistryInfo.PullPassword && oldState.RegistryInfo.IsInternal() {
if newState.RegistryInfo.PullPassword, err = helpers.RandomString(types.ZarfGeneratedPasswordLen); err != nil {
return nil, fmt.Errorf("%s: %w", lang.ErrUnableToGenerateRandomSecret, err)
}
Expand All @@ -330,21 +323,13 @@ func MergeZarfState(oldState *types.ZarfState, initOptions types.ZarfInitOptions
// TODO: Replace use of reflections with explicit setting
newState.GitServer = helpers.MergeNonZero(newState.GitServer, initOptions.GitServer)

// Set the state of the internal git server if it has changed
// TODO: Internal server should be a function of the address and not a property.
if newState.GitServer.Address == types.ZarfInClusterGitServiceURL {
newState.GitServer.InternalServer = true
} else {
newState.GitServer.InternalServer = false
}

// Set the new passwords if they should be autogenerated
if newState.GitServer.PushPassword == oldState.GitServer.PushPassword && oldState.GitServer.InternalServer {
if newState.GitServer.PushPassword == oldState.GitServer.PushPassword && oldState.GitServer.IsInternal() {
if newState.GitServer.PushPassword, err = helpers.RandomString(types.ZarfGeneratedPasswordLen); err != nil {
return nil, fmt.Errorf("%s: %w", lang.ErrUnableToGenerateRandomSecret, err)
}
}
if newState.GitServer.PullPassword == oldState.GitServer.PullPassword && oldState.GitServer.InternalServer {
if newState.GitServer.PullPassword == oldState.GitServer.PullPassword && oldState.GitServer.IsInternal() {
if newState.GitServer.PullPassword, err = helpers.RandomString(types.ZarfGeneratedPasswordLen); err != nil {
return nil, fmt.Errorf("%s: %w", lang.ErrUnableToGenerateRandomSecret, err)
}
Expand All @@ -354,16 +339,8 @@ func MergeZarfState(oldState *types.ZarfState, initOptions types.ZarfInitOptions
// TODO: Replace use of reflections with explicit setting
newState.ArtifactServer = helpers.MergeNonZero(newState.ArtifactServer, initOptions.ArtifactServer)

// Set the state of the internal artifact server if it has changed
// TODO: Internal server should be a function of the address and not a property.
if newState.ArtifactServer.Address == types.ZarfInClusterArtifactServiceURL {
newState.ArtifactServer.InternalServer = true
} else {
newState.ArtifactServer.InternalServer = false
}

// Set an empty token if it should be autogenerated
if newState.ArtifactServer.PushToken == oldState.ArtifactServer.PushToken && oldState.ArtifactServer.InternalServer {
if newState.ArtifactServer.PushToken == oldState.ArtifactServer.PushToken && oldState.ArtifactServer.IsInternal() {
newState.ArtifactServer.PushToken = ""
}
}
Expand Down
171 changes: 64 additions & 107 deletions src/pkg/cluster/state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,59 +199,46 @@ func TestMergeZarfStateRegistry(t *testing.T) {
{
name: "internal server auto generate",
oldRegistry: types.RegistryInfo{
Address: fmt.Sprintf("%s:%d", helpers.IPV4Localhost, 1),
NodePort: 1,
InternalRegistry: true,
Address: fmt.Sprintf("%s:%d", helpers.IPV4Localhost, 1),
NodePort: 1,
},
expectedRegistry: types.RegistryInfo{
Address: fmt.Sprintf("%s:%d", helpers.IPV4Localhost, 1),
NodePort: 1,
InternalRegistry: true,
Address: fmt.Sprintf("%s:%d", helpers.IPV4Localhost, 1),
NodePort: 1,
},
},
{
name: "external server",
name: "init options merged",
oldRegistry: types.RegistryInfo{
Address: "example.com",
InternalRegistry: false,
PushPassword: "push",
PullPassword: "pull",
},
expectedRegistry: types.RegistryInfo{
Address: "example.com",
InternalRegistry: false,
PushPassword: "push",
PullPassword: "pull",
PushUsername: "doesn't matter",
PullUsername: "doesn't matter",
Address: "doesn't matter",
NodePort: 0,
Secret: "doesn't matter",
},
},
{
name: "init options merged",
initRegistry: types.RegistryInfo{
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
NodePort: 1,
InternalRegistry: false,
Secret: "secret",
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
NodePort: 1,
Secret: "secret",
},
expectedRegistry: types.RegistryInfo{
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
NodePort: 1,
InternalRegistry: false,
Secret: "secret",
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
NodePort: 1,
Secret: "secret",
},
},
{
name: "init options not merged",
expectedRegistry: types.RegistryInfo{
PushUsername: "",
PullUsername: "",
Address: "",
NodePort: 0,
InternalRegistry: false,
Secret: "",
PushUsername: "",
PullUsername: "",
Address: "",
NodePort: 0,
Secret: "",
},
},
}
Expand All @@ -269,7 +256,6 @@ func TestMergeZarfStateRegistry(t *testing.T) {
require.Equal(t, tt.expectedRegistry.PullUsername, newState.RegistryInfo.PullUsername)
require.Equal(t, tt.expectedRegistry.Address, newState.RegistryInfo.Address)
require.Equal(t, tt.expectedRegistry.NodePort, newState.RegistryInfo.NodePort)
require.Equal(t, tt.expectedRegistry.InternalRegistry, newState.RegistryInfo.InternalRegistry)
require.Equal(t, tt.expectedRegistry.Secret, newState.RegistryInfo.Secret)
})
}
Expand All @@ -286,64 +272,51 @@ func TestMergeZarfStateGit(t *testing.T) {
expectedGitServer types.GitServerInfo
}{
{
name: "username is unmodified",
name: "address and usernames are unmodified",
oldGitServer: types.GitServerInfo{
Address: "address",
PushUsername: "push-user",
PullUsername: "pull-user",
},
expectedGitServer: types.GitServerInfo{
Address: "address",
PushUsername: "push-user",
PullUsername: "pull-user",
},
},
{
name: "internal server auto generate",
oldGitServer: types.GitServerInfo{
Address: types.ZarfInClusterGitServiceURL,
InternalServer: true,
Address: types.ZarfInClusterGitServiceURL,
},
expectedGitServer: types.GitServerInfo{
Address: types.ZarfInClusterGitServiceURL,
InternalServer: true,
Address: types.ZarfInClusterGitServiceURL,
},
},
{
name: "external server",
name: "init options merged",
oldGitServer: types.GitServerInfo{
Address: "example.com",
InternalServer: false,
PushPassword: "push",
PullPassword: "pull",
},
expectedGitServer: types.GitServerInfo{
Address: "example.com",
InternalServer: false,
PushPassword: "push",
PullPassword: "pull",
Address: "doesn't matter",
PushUsername: "doesn't matter",
PullUsername: "doesn't matter",
},
},
{
name: "init options merged",
initGitServer: types.GitServerInfo{
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
InternalServer: false,
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
},
expectedGitServer: types.GitServerInfo{
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
InternalServer: false,
PushUsername: "push-user",
PullUsername: "pull-user",
Address: "address",
},
},
{
name: "empty init options not merged",
expectedGitServer: types.GitServerInfo{
PushUsername: "",
PullUsername: "",
Address: "",
InternalServer: false,
PushUsername: "",
PullUsername: "",
Address: "",
},
},
}
Expand All @@ -360,7 +333,6 @@ func TestMergeZarfStateGit(t *testing.T) {
require.Equal(t, tt.expectedGitServer.PushUsername, newState.GitServer.PushUsername)
require.Equal(t, tt.expectedGitServer.PullUsername, newState.GitServer.PullUsername)
require.Equal(t, tt.expectedGitServer.Address, newState.GitServer.Address)
require.Equal(t, tt.expectedGitServer.InternalServer, newState.GitServer.InternalServer)
})
}
}
Expand All @@ -386,14 +358,12 @@ func TestMergeZarfStateArtifact(t *testing.T) {
{
name: "old state is internal server auto generate push token",
oldArtifactServer: types.ArtifactServerInfo{
PushToken: "foobar",
Address: types.ZarfInClusterArtifactServiceURL,
InternalServer: true,
PushToken: "foobar",
Address: types.ZarfInClusterArtifactServiceURL,
},
expectedArtifactServer: types.ArtifactServerInfo{
PushToken: "",
Address: types.ZarfInClusterArtifactServiceURL,
InternalServer: true,
PushToken: "",
Address: types.ZarfInClusterArtifactServiceURL,
},
},
{
Expand All @@ -402,51 +372,38 @@ func TestMergeZarfStateArtifact(t *testing.T) {
PushToken: "hello world",
},
oldArtifactServer: types.ArtifactServerInfo{
PushToken: "foobar",
Address: types.ZarfInClusterArtifactServiceURL,
InternalServer: false,
PushToken: "foobar",
Address: types.ZarfInClusterArtifactServiceURL,
},
expectedArtifactServer: types.ArtifactServerInfo{
PushToken: "hello world",
Address: types.ZarfInClusterArtifactServiceURL,
InternalServer: true,
PushToken: "hello world",
Address: types.ZarfInClusterArtifactServiceURL,
},
},
{
name: "external server same push token",
name: "init options merged",
oldArtifactServer: types.ArtifactServerInfo{
PushToken: "foobar",
Address: "http://example.com",
InternalServer: false,
},
expectedArtifactServer: types.ArtifactServerInfo{
PushToken: "foobar",
Address: "http://example.com",
InternalServer: false,
PushUsername: "doesn't matter",
PushToken: "doesn't matter",
Address: "doesn't matter",
},
},
{
name: "init options merged",
initArtifactServer: types.ArtifactServerInfo{
PushUsername: "user",
PushToken: "token",
Address: "address",
InternalServer: false,
PushUsername: "user",
PushToken: "token",
Address: "address",
},
expectedArtifactServer: types.ArtifactServerInfo{
PushUsername: "user",
PushToken: "token",
Address: "address",
InternalServer: false,
PushUsername: "user",
PushToken: "token",
Address: "address",
},
},
{
name: "empty init options not merged",
expectedArtifactServer: types.ArtifactServerInfo{
PushUsername: "",
PushToken: "",
Address: "",
InternalServer: false,
PushUsername: "",
PushToken: "",
Address: "",
},
},
}
Expand Down
Loading

0 comments on commit e0a9df5

Please sign in to comment.