AIDE migration #537

Vincent056 · 2024-07-02T14:10:12Z

This PR adds the ability to run aide 0.18 config checks and migration.

The aide0.18 will be compiled as an additional binary and used by FIO to check our migrated config. This new binary will reside in the container image, and we will launch a new pod to check the aide config migration every time we need to re-init.

Add config check to existing, upgrade case, check if migration checks needed annotation, if so we launch the pod, pod checks for aide migration and issue warning return result as a config map, the config map controller pick up the change, remove migration checks needed annotation.

We will try to perform the migration if a user-defined config is detected. We will issue warning messages in the log, and have a failed annotation key in the FIO instance if we cannot pre-migrate the config.

Annotation keys added:

	// AideConfigMigrationIgnoreAnnotationKey tells us to ignore the deprecated config options
	AideConfigMigrationIgnoreAnnotationKey = "file-integrity.openshift.io/migration-ignore-deprecation"
	// AideConfigMigrationFailedAnnotationKey tells us that the migration failed
	AideConfigAutoMigrationFailedAnnotationKey = "file-integrity.openshift.io/migration-failed"
	// AideConfigMigrationCheckDisabledAnnotationKey tells us that the migration check is disabled
	AideConfigMigrationCheckDisabledAnnotationKey = "file-integrity.openshift.io/migration-check-disabled"

example of event message:

7m17s       Warning   FileIntegrityAIDEConfigMigration   fileintegrity/example-fileintegrity            Migration check failed: Detected error config during the migration check: Invalid configureline error

AIDE Configuration Changes

Only contains things that are being deprecated or removed along with their potential replacement

Removed Features in AIDE v0.17

ignore_list
- Attributes whose changes are ignored in the report.
report_attributes
- Attributes always printed in the report for changed files. If an attribute is both ignored and forced, it is not considered for file change but will be printed in the final report if the file has otherwise changed.
verbose (type: number, range: 0 - 255, default: 5)
- Removed, use log_level and report_level options instead.

New Features in AIDE v0.17

database_in (type: URL, default: see \fB--version\fP output, added in AIDE v0.17)
log_level (type: log level, default: \fBwarning\fR)
report_level (type: report_level, default: \fBchanged_attributes\fR)
- Specifies the log level. Log messages are written to \fIstderr\fR. If there are multiple \fIlog_level\fR lines, the first one is used. The --log-level or -L command line option overwrites this option.

New Features in AIDE v0.16

report_ignore_added_attrs (type: attribute expression, default: empty)
- Attributes whose addition is ignored in the report.
report_ignore_removed_attrs (type: attribute expression, default: empty)
- Attributes whose removal is ignored in the report.
report_ignore_e2fsattrs (type: string, default: 0)
- List (no delimiter) of ext2 file attributes to be ignored in the report.
report_force_attrs (type: attribute expression, default: empty)
- Attributes always printed in the report for changed files.

Deprecated Features in AIDE v0.18 (to be removed in AIDE v0.20)

@@ifdef VARIABLE
- Same as @@if defined VARIABLE.
@@ifndef VARIABLE
- Same as @@if not defined VARIABLE.
@@ifhost HOSTNAME
- Same as @@if hostname HOSTNAME.
@@ifnhost HOSTNAME
- Same as @@if not hostname HOSTNAME.
Special attributes
- S
  - Check for growing size. (DEPRECATED since AIDE v0.18, will be removed in AIDE v0.20)
  - Use growing+s attributes instead.

Removed Features in AIDE v0.19

database
- The URL from which the database is read. Only one of these lines is allowed. If there are multiple database lines, the first is used.

openshift-ci · 2024-07-02T14:14:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Vincent056

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Vincent056]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Vincent056 · 2024-07-09T14:54:39Z

/retest

pkg/controller/fileintegrity/config_defaults.go

Vincent056 · 2024-07-12T18:45:07Z

we will save migrated config in:

kind: ConfigMap
apiVersion: v1
metadata:
  name: example-fileintegrity
  namespace: openshift-file-integrity
  uid: 3e68694f-fed1-403f-805f-813eec85ff38
  resourceVersion: '68362036'
  creationTimestamp: '2024-06-28T06:22:38Z'
  labels:
    file-integrity.openshift.io/aide-conf: ''
    file-integrity.openshift.io/owner: example-fileintegrity
  annotations:
    kubernetes.io/description: hi
  managedFields:
    - manager: Mozilla
      operation: Update
      apiVersion: v1
      time: '2024-07-12T06:37:37Z'
      fieldsType: FieldsV1
      fieldsV1:
        'f:metadata':
          'f:annotations':
            .: {}
            'f:kubernetes.io/description': {}
    - manager: file-integrity-operator
      operation: Update
      apiVersion: v1
      time: '2024-07-12T18:35:21Z'
      fieldsType: FieldsV1
      fieldsV1:
        'f:data':
          .: {}
          'f:aide-0.18.conf': {}
          'f:aide.conf': {}
        'f:metadata':
          'f:labels':
            .: {}
            'f:file-integrity.openshift.io/aide-conf': {}
            'f:file-integrity.openshift.io/owner': {}
data:
  aide-0.18.conf: |-
    @@define DBDIR /hostroot/etc/kubernetes
    @@define LOGDIR /hostroot/etc/kubernetes
    database_in=file:@@{DBDIR}/aide.db.gz
    database_out=file:@@{DBDIR}/aide.db.gz.new
    gzip_dbout=yes
    log_level=notice
    report_level=added_removed_attributes
    report_url=file:@@{LOGDIR}/aide.log.new
    report_url=stdoutx
    PERMS = p+u+g+acl+selinux+xattrs
    CONTENTEX=sha512+ftype+p+u+g+n+acl+selinux+xattrs

    /hostroot/boot/        CONTENTEX
    /hostroot/root/\..* PERMS
    /hostroot/root/   CONTENTEX
    !/hostroot/root/\.kube
    !/hostroot/usr/src/
    !/hostroot/usr/tmp/

    /hostroot/usr/    CONTENTEX

    # OpenShift specific excludes
    !/hostroot/opt/
    !/hostroot/var
    !/hostroot/etc/NetworkManager/system-connections/
    !/hostroot/etc/mtab$
    !/hostroot/etc/.*~
    !/hostroot/etc/kubernetes/static-pod-resources
    !/hostroot/etc/kubernetes/test
    !/hostroot/etc/kubernetes/aide.*
    !/hostroot/etc/kubernetes/manifests
    !/hostroot/etc/kubernetes/kubelet-ca.crt
    !/hostroot/etc/docker/certs.d
    !/hostroot/etc/selinux/targeted
    !/hostroot/etc/openvswitch/conf.db
    !/hostroot/etc/kubernetes/cni/net.d
    !/hostroot/etc/kubernetes/cni/net.d/*
    !/hostroot/etc/machine-config-daemon/currentconfig$
    !/hostroot/etc/machine-config-daemon/node-annotation.json*
    !/hostroot/etc/pki/ca-trust/extracted/java/cacerts$
    !/hostroot/etc/cvo/updatepayloads
    !/hostroot/etc/cni/multus/certs
    !/hostroot/etc/kubernetes/compliance-operator
    !/hostroot/etc/kubernetes/node-feature-discovery

    # Catch everything else in /etc
    /hostroot/etc/    CONTENTEX
  aide.conf: |-
    @@define DBDIR /hostroot/etc/kubernetes
    @@define LOGDIR /hostroot/etc/kubernetes
    database=file:@@{DBDIR}/aide.db.gz
    database_out=file:@@{DBDIR}/aide.db.gz.new
    gzip_dbout=yes
    verbose=6
    report_url=file:@@{LOGDIR}/aide.log.new
    report_url=stdoutx
    PERMS = p+u+g+acl+selinux+xattrs
    CONTENT_EX = sha512+ftype+p+u+g+n+acl+selinux+xattrs

    /hostroot/boot/        CONTENT_EX
    /hostroot/root/\..* PERMS
    /hostroot/root/   CONTENT_EX
    !/hostroot/root/\.kube
    !/hostroot/usr/src/
    !/hostroot/usr/tmp/

    /hostroot/usr/    CONTENT_EX

    # OpenShift specific excludes
    !/hostroot/opt/
    !/hostroot/var
    !/hostroot/etc/NetworkManager/system-connections/
    !/hostroot/etc/mtab$
    !/hostroot/etc/.*~
    !/hostroot/etc/kubernetes/static-pod-resources
    !/hostroot/etc/kubernetes/test
    !/hostroot/etc/kubernetes/aide.*
    !/hostroot/etc/kubernetes/manifests
    !/hostroot/etc/kubernetes/kubelet-ca.crt
    !/hostroot/etc/docker/certs.d
    !/hostroot/etc/selinux/targeted
    !/hostroot/etc/openvswitch/conf.db
    !/hostroot/etc/kubernetes/cni/net.d
    !/hostroot/etc/kubernetes/cni/net.d/*
    !/hostroot/etc/machine-config-daemon/currentconfig$
    !/hostroot/etc/machine-config-daemon/node-annotation.json*
    !/hostroot/etc/pki/ca-trust/extracted/java/cacerts$
    !/hostroot/etc/cvo/updatepayloads
    !/hostroot/etc/cni/multus/certs
    !/hostroot/etc/kubernetes/compliance-operator
    !/hostroot/etc/kubernetes/node-feature-discovery

    # Catch everything else in /etc
    /hostroot/etc/    CONTENT_EX

Vincent056 · 2024-07-12T18:45:48Z

@rhmdnd this should be ready for some reviews

rhmdnd

Took an initial pass, but I still need to come back and look through the config map controller and file integrity controller logic.

Posting the feedback I have for now.

rhmdnd · 2024-07-12T21:05:32Z

Dockerfile.ci

 COPY . .

+RUN git clone https://github.com/autoconf-archive/autoconf-archive.git && \
+    cp autoconf-archive/m4/*.m4 /usr/share/aclocal/ 


Is this a dependency for building AIDE 0.18?

yes, I think rhel might has rpm for this, but this builder does not

rhmdnd · 2024-07-12T21:08:23Z

Makefile

+aide-0.18:
+	rm -rf aide && \
+	git clone https://github.com/aide/aide.git && \
+	cd aide && \


We have a build directory where we stash other related tools like controller-gen and kustomize. We could keep the entire repository in there so it gets cleaned up using make clean. Otherwise, we might need to go in and clean it up manually if we encounter a build failure.

Makefile

cmd/manager/daemon.go

cmd/manager/loops.go

rhmdnd · 2024-07-12T21:28:19Z

tests/e2e/e2e_test.go

+	err = waitForAIDEConfigMigrationEvent(t, f, namespace, testName, "17")
+	if err != nil {
+		t.Errorf("Expected event to be found")
+	}


Nice - thanks for adding this. So the remaining bit to test would be running AIDE 0.18.0, yeah? At this point, the test is assertion the configuration should work with 0.18.0, but we're still running 0.16.0.

Vincent056 · 2024-07-19T03:23:59Z

Took an initial pass, but I still need to come back and look through the config map controller and file integrity controller logic.

Posting the feedback I have for now.

thanks for the detail reviews!

This PR adds ability to run aide 0.18 config checks and migration. The aide0.18 will be complied as binary and used by FIO in a container, we will try to see if we can perform the migration if a user defined config is being detected. We will issue warning messages in the log, and have a failed annotation key in the FIO instance if we are not able to pre migrate the config.

Vincent056 · 2024-07-23T01:08:07Z

/retest

Vincent056 · 2024-07-23T05:08:05Z

/retest

openshift-ci · 2024-07-23T07:39:32Z

@Vincent056: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

rhmdnd

Looking good - a few more comments inline now that I've had more time to walk through the controller code and the tests.

rhmdnd · 2024-07-23T16:38:32Z

cmd/manager/loops.go

+	for {
+		select {
+		case <-migrateCtx.Done():
+			DBG("Migration loop cancelled by the main routine!")


In this case - has the operation been aborted due to an error? Can, or should, the user do anything to restart the migration?

rhmdnd · 2024-07-23T16:40:29Z

cmd/manager/loops.go

+				return
+			} else if aideResult == 17 {
+				// This is an AIDE config line error.
+				newErr := fmt.Sprintf("Detected configuration error during the migration check for AIDE 0.18: %s, output: %s ", common.GetAideErrorMessage(aideResult), output)


Cool - this error moves us in the right direction I think by giving users the information they need to update AIDE configs. Curious to see how this renders in the logs with an actual AIDE error.

rhmdnd · 2024-07-23T16:44:29Z

pkg/common/constants.go

-	ReinitDaemonSetPrefix              = "aide-ini"
+
+	// AideConfigMigrationIgnoreAnnotationKey tells us to ignore the deprecated config options
+	AideConfigMigrationIgnoreAnnotationKey = "file-integrity.openshift.io/migration-ignore-deprecation"


nit: For additional clarity, we're not ignoring potential FIO deprecations, but AIDE deprecations. We could update that in the annotation to be more explicit:

AideConfigMigrationIgnoreAnnotationKey = "file-integrity.openshift.io/migration-ignore-aide-deprecation"

rhmdnd · 2024-07-23T16:47:34Z

pkg/controller/configmap/configmap_controller.go

+func (r *ReconcileConfigMap) handleMigrationCheckLog(cm *corev1.ConfigMap, logger logr.Logger) (reconcile.Result, error) {
+	owner, err := common.GetConfigMapOwnerName(cm)
+	if err != nil {
+		logger.Error(err, "Malformed ConfigMap: Could not get owner. Cannot retry.")


In this case, we need the user to go adjust the ConfigMap manually, right? Can we include the error in the log output, or does it become unruly?

rhmdnd · 2024-07-23T16:50:28Z

pkg/controller/configmap/configmap_controller.go

+		delete(annotation, common.IntegrityLogErrorAnnotationKey)
+	}
+
+	delete(annotation, common.IntegrityMigrationUpdateAnnotationKey)


We delete this annotation because the migration has been successful?

rhmdnd · 2024-07-23T20:36:52Z

build/Dockerfile

+
 RUN make build

+RUN make aide-0.18


Another alternative we talked about earlier was to build this using a separate ubi8 base image container and then copying it over, too.

Or, at least in this case, we could just copy it out of the fedora 40 image and see how that works.

rhmdnd · 2024-07-23T20:38:14Z

build/Dockerfile

 # install operator binary
 COPY --from=builder /go/src/github.com/openshift/file-integrity-operator/build/bin/manager ${OPERATOR}
 COPY build/bin /usr/local/bin
+COPY --from=builder /go/src/github.com/openshift/file-integrity-operator/build/bin/aide-0.18 /usr/sbin/aide-0.18


Then we don't need to pollute the builder container image with utilities and code unrelated to golang, or building FIO.

rhmdnd · 2024-07-23T20:45:05Z

tests/e2e/helpers.go

I think we'll also need to update the default AIDE configuration.

https://github.com/openshift/file-integrity-operator/blob/master/pkg/controller/fileintegrity/config_defaults.go#L15-L61

The way I understand the current logic is that FIO will continue to lay down AIDE configurations that are not compatible with 0.18.0. I think we'll want to change that as soon as possible, so long as the default AIDE configuration for 0.18.0 we want to use works on 0.16.0. That way we don't have the maintain the migration logic any longer than we absolutely have to.

rhmdnd · 2024-07-23T20:50:06Z

tests/e2e/helpers.go

 		return
 	}

-	// Append operatorPods to pods


nit: unrelated change?

rhmdnd · 2024-07-23T20:53:35Z

tests/e2e/helpers.go

 	return nil
 }

+func waitForFIMigrationErrAnnotation(t *testing.T, f *framework.Framework, namespace, name, expectedMessage string) error {


nit: we could use this as an assertion and not have to deal with checking the error output in the caller to fail the test, making it more of an assertion.

func assertAIDEMigrationEmitsAnAlert(...)

And then a similar one below:

func assertAIDEMigrationIsSuccessful(...)

But, I acknowledge this would be potentially different from existing conventions and we could do that in a separate PR if we decide to make that change across the entire functional test suite.

openshift-bot · 2024-10-23T01:00:25Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-merge-robot · 2024-10-23T01:00:36Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-bot · 2024-11-22T08:30:52Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2024-12-23T00:00:47Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2024-12-23T00:01:25Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci bot requested review from BhargaviGudi and mkumku July 2, 2024 14:14

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 2, 2024

Vincent056 force-pushed the aide-migrate branch 4 times, most recently from 2a2f984 to 2e48413 Compare July 2, 2024 17:12

Vincent056 requested review from rhmdnd, xiaojiey and yuumasato July 2, 2024 18:31

Vincent056 force-pushed the aide-migrate branch from 2e48413 to ff5ed41 Compare July 9, 2024 12:25

rhmdnd reviewed Jul 10, 2024

View reviewed changes

pkg/controller/fileintegrity/config_defaults.go Show resolved Hide resolved

Vincent056 force-pushed the aide-migrate branch 5 times, most recently from 573bd4f to 2d23d2a Compare July 12, 2024 18:44

Vincent056 force-pushed the aide-migrate branch 4 times, most recently from 24fed09 to 130dbd0 Compare July 12, 2024 19:22

rhmdnd requested changes Jul 12, 2024

View reviewed changes

openshift-ci bot assigned rhmdnd Jul 12, 2024

Vincent056 force-pushed the aide-migrate branch 2 times, most recently from 03f6255 to a2ffd69 Compare July 22, 2024 06:39

Vincent056 force-pushed the aide-migrate branch 2 times, most recently from 8b33a9f to e8d41fc Compare July 22, 2024 23:18

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2024

Vincent056 force-pushed the aide-migrate branch from e8d41fc to b387f5e Compare July 22, 2024 23:20

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2024

Vincent056 force-pushed the aide-migrate branch from b387f5e to 8a4a937 Compare July 22, 2024 23:36

Vincent056 requested a review from rhmdnd July 23, 2024 01:08

rhmdnd requested changes Jul 23, 2024

View reviewed changes

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 23, 2024

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2024

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 22, 2024

openshift-ci bot closed this Dec 23, 2024

AIDE migration #537

AIDE migration #537

Uh oh!

Conversation

Vincent056 commented Jul 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AIDE Configuration Changes

Removed Features in AIDE v0.17

New Features in AIDE v0.17

New Features in AIDE v0.16

Deprecated Features in AIDE v0.18 (to be removed in AIDE v0.20)

Removed Features in AIDE v0.19

Uh oh!

openshift-ci bot commented Jul 2, 2024

Uh oh!

Vincent056 commented Jul 9, 2024

Uh oh!

Uh oh!

Vincent056 commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vincent056 commented Jul 12, 2024

Uh oh!

rhmdnd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vincent056 commented Jul 19, 2024

Uh oh!

Vincent056 commented Jul 23, 2024

Uh oh!

Vincent056 commented Jul 23, 2024

Uh oh!

openshift-ci bot commented Jul 23, 2024

Uh oh!

rhmdnd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Oct 23, 2024

Uh oh!

openshift-merge-robot commented Oct 23, 2024

Uh oh!

openshift-bot commented Nov 22, 2024

Uh oh!

Vincent056 commented Jul 2, 2024 •

edited

Loading

Vincent056 commented Jul 12, 2024 •

edited

Loading