Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: change isInternal variables to functions #2768

Merged
merged 15 commits into from
Aug 5, 2024
2 changes: 1 addition & 1 deletion src/cmd/internal.go
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
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
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
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
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
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
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