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

ROX-18504: GitOps Cleanup #1418

Merged
merged 9 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@
"filename": "internal/dinosaur/pkg/api/public/api/openapi.yaml",
"hashed_secret": "5b455797b93de5b6a19633ba22127c8a610f5c1b",
"is_verified": false,
"line_number": 1663
"line_number": 1546
}
],
"pkg/client/iam/client_moq.go": [
Expand Down Expand Up @@ -564,5 +564,5 @@
}
]
},
"generated_at": "2023-10-23T12:42:41Z"
"generated_at": "2023-10-26T16:03:47Z"
}
2 changes: 1 addition & 1 deletion cmd/fleet-manager/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestInjections(t *testing.T) {

var bootList []environments.BootService
env.MustResolve(&bootList)
Expect(len(bootList)).To(Equal(7))
Expect(len(bootList)).To(Equal(6))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be because we are removing the DefaultVersionService from the di container


_, ok := bootList[0].(*server.APIServer)
Expect(ok).To(Equal(true))
Expand Down
113 changes: 4 additions & 109 deletions fleetshard/pkg/central/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"net/url"
"reflect"
"sort"
"strconv"
"sync/atomic"
"time"

Expand All @@ -27,7 +26,6 @@ import (
"github.com/stackrox/acs-fleet-manager/fleetshard/pkg/util"
centralConstants "github.com/stackrox/acs-fleet-manager/internal/dinosaur/constants"
"github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/api/private"
"github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/converters"
"github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager"
"github.com/stackrox/acs-fleet-manager/pkg/features"
"github.com/stackrox/rox/operator/apis/platform/v1alpha1"
Expand Down Expand Up @@ -271,113 +269,13 @@ func (r *CentralReconciler) Reconcile(ctx context.Context, remoteCentral private
}

func (r *CentralReconciler) getInstanceConfig(remoteCentral *private.ManagedCentral) (*v1alpha1.Central, error) {
var central *v1alpha1.Central
var err error
if r.shouldUseGitopsConfig(remoteCentral) {
if central, err = r.getInstanceConfigWithGitops(remoteCentral); err != nil {
return nil, err
}
} else {
if central, err = r.getLegacyInstanceConfig(remoteCentral); err != nil {
return nil, err
}
}
if err := r.applyCentralConfig(remoteCentral, central); err != nil {
return nil, err
}
return central, nil
}

func (r *CentralReconciler) shouldUseGitopsConfig(remoteCentral *private.ManagedCentral) bool {
return features.GitOpsCentrals.Enabled() && len(remoteCentral.Spec.CentralCRYAML) > 0
}

func (r *CentralReconciler) getInstanceConfigWithGitops(remoteCentral *private.ManagedCentral) (*v1alpha1.Central, error) {
var central = new(v1alpha1.Central)
if err := yaml2.Unmarshal([]byte(remoteCentral.Spec.CentralCRYAML), central); err != nil {
return nil, errors.Wrap(err, "failed to unmarshal central yaml")
}
return central, nil
}

func (r *CentralReconciler) getLegacyInstanceConfig(remoteCentral *private.ManagedCentral) (*v1alpha1.Central, error) {
if remoteCentral == nil {
return nil, errInvalidArguments
}

remoteCentralName := remoteCentral.Metadata.Name
remoteCentralNamespace := remoteCentral.Metadata.Namespace

monitoringExposeEndpointEnabled := v1alpha1.ExposeEndpointEnabled
centralResources, err := converters.ConvertPrivateResourceRequirementsToCoreV1(&remoteCentral.Spec.Central.Resources)
if err != nil {
return nil, errors.Wrap(err, "converting Central resources")
}
scannerAnalyzerResources, err := converters.ConvertPrivateResourceRequirementsToCoreV1(&remoteCentral.Spec.Scanner.Analyzer.Resources)
if err != nil {
return nil, errors.Wrap(err, "converting Scanner Analyzer resources")
}
scannerAnalyzerScaling := converters.ConvertPrivateScalingToV1(&remoteCentral.Spec.Scanner.Analyzer.Scaling)
scannerDbResources, err := converters.ConvertPrivateResourceRequirementsToCoreV1(&remoteCentral.Spec.Scanner.Db.Resources)
if err != nil {
return nil, errors.Wrap(err, "converting Scanner DB resources")
}

scannerComponentEnabled := v1alpha1.ScannerComponentEnabled

central := &v1alpha1.Central{
ObjectMeta: metav1.ObjectMeta{
Name: remoteCentralName,
Namespace: remoteCentralNamespace,
Labels: map[string]string{
k8s.ManagedByLabelKey: k8s.ManagedByFleetshardValue,
tenantIDLabelKey: remoteCentral.Id,
instanceTypeLabelKey: remoteCentral.Spec.Central.InstanceType,
orgIDLabelKey: remoteCentral.Spec.Auth.OwnerOrgId,
},
Annotations: map[string]string{
centralPVCAnnotationKey: strconv.FormatBool(r.managedDBEnabled),
managedServicesAnnotation: "true",
orgNameAnnotationKey: remoteCentral.Spec.Auth.OwnerOrgName,
},
},
Spec: v1alpha1.CentralSpec{
Central: &v1alpha1.CentralComponentSpec{
Monitoring: &v1alpha1.Monitoring{
ExposeEndpoint: &monitoringExposeEndpointEnabled,
},
DeploymentSpec: v1alpha1.DeploymentSpec{
Resources: &centralResources,
},
},
Scanner: &v1alpha1.ScannerComponentSpec{
Analyzer: &v1alpha1.ScannerAnalyzerComponent{
DeploymentSpec: v1alpha1.DeploymentSpec{
Resources: &scannerAnalyzerResources,
},
Scaling: &scannerAnalyzerScaling,
},
DB: &v1alpha1.DeploymentSpec{
Resources: &scannerDbResources,
},
Monitoring: &v1alpha1.Monitoring{
ExposeEndpoint: &monitoringExposeEndpointEnabled,
},
ScannerComponent: &scannerComponentEnabled,
},
Customize: &v1alpha1.CustomizeSpec{
Annotations: map[string]string{
orgNameAnnotationKey: remoteCentral.Spec.Auth.OwnerOrgName,
},
Labels: map[string]string{
orgIDLabelKey: remoteCentral.Spec.Auth.OwnerOrgId,
tenantIDLabelKey: remoteCentral.Id,
instanceTypeLabelKey: remoteCentral.Spec.Central.InstanceType,
},
},
},
if err := r.applyCentralConfig(remoteCentral, central); err != nil {
return nil, err
}

return central, nil
}

Expand Down Expand Up @@ -1695,11 +1593,8 @@ func (r *CentralReconciler) needsReconcile(changed bool, remoteCentral private.M
if changed {
return true
}
if r.shouldUseGitopsConfig(&remoteCentral) {
forceReconcile, ok := central.Labels["rhacs.redhat.com/force-reconcile"]
return ok && forceReconcile == "true"
}
return remoteCentral.ForceReconcile == "always"
forceReconcile, ok := central.Labels["rhacs.redhat.com/force-reconcile"]
return ok && forceReconcile == "true"
}

var resourcesChart = charts.MustGetChart("tenant-resources", nil)
Expand Down
142 changes: 75 additions & 67 deletions fleetshard/pkg/central/reconciler/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/base64"
"fmt"
"net/http"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -56,8 +55,6 @@ const (
conditionTypeReady = "Ready"
clusterName = "test-cluster"
environment = "test"
operatorVersion = "4.0.1"
operatorImage = "quay.io/rhacs-eng/stackrox-operator:" + operatorVersion
)

var (
Expand Down Expand Up @@ -113,10 +110,12 @@ var simpleManagedCentral = private.ManagedCentral{
DataEndpoint: private.ManagedCentralAllOfSpecDataEndpoint{
Host: fmt.Sprintf("acs-data-%s.acs.rhcloud.test", centralID),
},
Central: private.ManagedCentralAllOfSpecCentral{
InstanceType: "standard",
},
OperatorImage: operatorImage,
InstanceType: "standard",
CentralCRYAML: `
metadata:
name: ` + centralName + `
namespace: ` + centralNamespace + `
`,
},
}

Expand Down Expand Up @@ -206,16 +205,7 @@ func TestReconcileCreate(t *testing.T) {
err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: centralName, Namespace: centralNamespace}, central)
require.NoError(t, err)
assert.Equal(t, centralName, central.GetName())
assert.Equal(t, simpleManagedCentral.Id, central.GetLabels()[tenantIDLabelKey])
assert.Equal(t, simpleManagedCentral.Id, central.Spec.Customize.Labels[tenantIDLabelKey])
assert.Equal(t, environment, central.Spec.Customize.Annotations[envAnnotationKey])
assert.Equal(t, clusterName, central.Spec.Customize.Annotations[clusterNameAnnotationKey])
assert.Equal(t, simpleManagedCentral.Spec.Auth.OwnerOrgName, central.Spec.Customize.Annotations[orgNameAnnotationKey])
assert.Equal(t, simpleManagedCentral.Spec.Auth.OwnerOrgId, central.Spec.Customize.Labels[orgIDLabelKey])
assert.Equal(t, simpleManagedCentral.Spec.Central.InstanceType, central.Spec.Customize.Labels[instanceTypeLabelKey])
assert.Equal(t, "1", central.GetAnnotations()[util.RevisionAnnotationKey])
assert.Equal(t, "false", central.GetAnnotations()[centralPVCAnnotationKey])
assert.Equal(t, "true", central.GetAnnotations()[managedServicesAnnotation])
assert.Equal(t, true, *central.Spec.Central.Exposure.Route.Enabled)

route := &openshiftRouteV1.Route{}
Expand Down Expand Up @@ -261,7 +251,6 @@ func TestReconcileCreateWithManagedDB(t *testing.T) {
central := &v1alpha1.Central{}
err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: centralName, Namespace: centralNamespace}, central)
require.NoError(t, err)
assert.Equal(t, "true", central.GetAnnotations()[centralPVCAnnotationKey])

route := &openshiftRouteV1.Route{}
err = fakeClient.Get(context.TODO(), client.ObjectKey{Name: centralReencryptRouteName, Namespace: centralNamespace}, route)
Expand Down Expand Up @@ -478,7 +467,15 @@ func TestIgnoreCacheForCentralForceReconcileAlways(t *testing.T) {

managedCentral := simpleManagedCentral
managedCentral.RequestStatus = centralConstants.CentralRequestStatusReady.String()
managedCentral.ForceReconcile = "always"
managedCentral.Spec.CentralCRYAML = `
metadata:
name: ` + centralName + `
namespace: ` + centralNamespace + `
annotations:
stackrox.io/force-reconcile: "true"
spec:
central: {}
`

expectedHash, err := util.MD5SumFromJSONStruct(&managedCentral)
require.NoError(t, err)
Expand Down Expand Up @@ -2137,7 +2134,7 @@ func TestReconciler_applyAnnotations(t *testing.T) {
}, c.Spec.Customize.Annotations)
}

func TestReconciler_getInstanceConfigWithGitops(t *testing.T) {
func TestReconciler_getInstanceConfig(t *testing.T) {

tcs := []struct {
name string
Expand All @@ -2157,7 +2154,6 @@ kind: Central
metadata:
name: central
namespace: rhacs
spec: {}
`,
expectCentral: &v1alpha1.Central{
TypeMeta: metav1.TypeMeta{
Expand All @@ -2168,6 +2164,64 @@ spec: {}
Name: "central",
Namespace: "rhacs",
},
Spec: v1alpha1.CentralSpec{
Central: &v1alpha1.CentralComponentSpec{
Exposure: &v1alpha1.Exposure{
Route: &v1alpha1.ExposureRoute{
Enabled: pointer.Bool(false),
},
},
DeclarativeConfiguration: &v1alpha1.DeclarativeConfiguration{
Secrets: []v1alpha1.LocalSecretReference{
{
Name: "cloud-service-sensible-declarative-configs",
}, {
Name: "cloud-service-manual-declarative-configs",
},
},
},
Telemetry: &v1alpha1.Telemetry{
Enabled: pointer.Bool(false),
Storage: &v1alpha1.TelemetryStorage{
Endpoint: pointer.String(""),
Key: pointer.String(""),
},
},
},
Customize: &v1alpha1.CustomizeSpec{
Annotations: map[string]string{
"rhacs.redhat.com/environment": "",
"rhacs.redhat.com/cluster-name": "",
},
EnvVars: []v1.EnvVar{
{
Name: "http_proxy",
Value: "http://egress-proxy.rhacs.svc:3128",
}, {
Name: "HTTP_PROXY",
Value: "http://egress-proxy.rhacs.svc:3128",
}, {
Name: "https_proxy",
Value: "http://egress-proxy.rhacs.svc:3128",
}, {
Name: "HTTPS_PROXY",
Value: "http://egress-proxy.rhacs.svc:3128",
}, {
Name: "all_proxy",
Value: "http://egress-proxy.rhacs.svc:3128",
}, {
Name: "ALL_PROXY",
Value: "http://egress-proxy.rhacs.svc:3128",
}, {
Name: "no_proxy",
Value: ":0,central.rhacs.svc:443,central.rhacs:443,central:443,kubernetes.default.svc.cluster.local.:443,scanner-db.rhacs.svc:5432,scanner-db.rhacs:5432,scanner-db:5432,scanner.rhacs.svc:8080,scanner.rhacs.svc:8443,scanner.rhacs:8080,scanner.rhacs:8443,scanner:8080,scanner:8443",
}, {
Name: "NO_PROXY",
Value: ":0,central.rhacs.svc:443,central.rhacs:443,central:443,kubernetes.default.svc.cluster.local.:443,scanner-db.rhacs.svc:5432,scanner-db.rhacs:5432,scanner-db:5432,scanner.rhacs.svc:8080,scanner.rhacs.svc:8443,scanner.rhacs:8080,scanner.rhacs:8443,scanner:8080,scanner:8443",
},
},
},
},
},
},
}
Expand All @@ -2180,7 +2234,7 @@ spec: {}
CentralCRYAML: tc.yaml,
},
}
c, err := r.getInstanceConfigWithGitops(mc)
c, err := r.getInstanceConfig(mc)
if tc.expectErr {
assert.Error(t, err)
} else {
Expand All @@ -2191,49 +2245,3 @@ spec: {}
}

}

func TestReconciler_shouldUseGitopsConfig(t *testing.T) {
tcs := []struct {
name string
managedCentral *private.ManagedCentral
featureFlagEnabled bool
expect bool
}{
{
name: "should return true when centralCRYAML is set and feature flag is enabled",
managedCentral: &private.ManagedCentral{
Spec: private.ManagedCentralAllOfSpec{
CentralCRYAML: "foo",
},
},
featureFlagEnabled: true,
expect: true,
}, {
name: "should return false when centralCRYAML is set and feature flag is disabled",
managedCentral: &private.ManagedCentral{
Spec: private.ManagedCentralAllOfSpec{
CentralCRYAML: "foo",
},
},
featureFlagEnabled: false,
expect: false,
}, {
name: "should return false when centralCRYAML is not set and feature flag is enabled",
managedCentral: &private.ManagedCentral{
Spec: private.ManagedCentralAllOfSpec{
CentralCRYAML: "",
},
},
featureFlagEnabled: true,
expect: false,
},
}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
t.Setenv("RHACS_GITOPS_ENABLED", strconv.FormatBool(tc.featureFlagEnabled))
r := &CentralReconciler{}
assert.Equal(t, tc.expect, r.shouldUseGitopsConfig(tc.managedCentral))
})
}
}
Loading
Loading