Skip to content

Conversation

@tanvipenumudy
Copy link
Contributor

@tanvipenumudy tanvipenumudy commented Mar 5, 2024

What changes were proposed in this pull request?

We should refine audit logging for operations modifying bucket properties.

How can this be useful?

  • Critical for consumers on earlier versions of Ozone who could potentially run into known bugs: HDDS-7449 and HDDS-7526.
  • Losing bucket replication properties/bucket encryption properties when one (re)sets quota/bucket replication configurations poses significant risks.
  • It is difficult for diagnosing the root cause when one runs into such issues just by looking at the audit logs.
  • Currently, the audit logs do not provide much insight into what properties have been modified while performing bucket config re(set) operations.

As of today, we are only capturing basic information such as volume, bucket, gdprEnabled, isVersionEnabled, storageType and owner properties for any given bucket.

We should also be capturing bucket quota, encryption and replication-related properties.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10460

How was this patch tested?

Create a bucket with replication-type: RATIS, replication-factor: THREE:

ozone sh bucket create voltest/bucktest --type=RATIS --replication=THREE

Set quotaInBytes, quotaInNamespace for the created bucket:

ozone sh bucket setquota --quota=1GB --namespace-quota=5 voltest/bucktest

om-audit.log:

2024-03-05 12:35:15,917 | INFO  | OMAudit | user=hadoop | ip=172.19.0.9 | op=UPDATE_BUCKET {volume=voltest, bucket=bucktest, gdprEnabled=null, isVersionEnabled=null, quotaInBytes=1073741824, quotaInNamespace=5} | ret=SUCCESS |

Set replication-config from replication-factor: THREE to replication-factor: ONE:

ozone sh bucket set-replication-config --type=RATIS --replication=ONE voltest/bucktest

om-audit.log:

2024-03-05 12:35:59,013 | INFO  | OMAudit | user=hadoop | ip=172.19.0.9 | op=UPDATE_BUCKET {volume=voltest, bucket=bucktest, gdprEnabled=null, isVersionEnabled=null, replicationType=RATIS, replicationFactor=ONE} | ret=SUCCESS |

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @tanvipenumudy for the patch.

tanvipenumudy and others added 3 commits March 5, 2024 16:35
…elpers/OmBucketArgs.java

Co-authored-by: Doroszlai, Attila <[email protected]>
…elpers/OmBucketArgs.java

Co-authored-by: Doroszlai, Attila <[email protected]>
…elpers/OmBucketArgs.java

Co-authored-by: Doroszlai, Attila <[email protected]>
@tanvipenumudy tanvipenumudy marked this pull request as ready for review March 5, 2024 13:37
Copy link
Contributor

@myskov myskov left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @tanvipenumudy , LGTM

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks again @tanvipenumudy for the patch.

On another look, similar change is needed in OmBucketInfo, which is used as source of audit data during bucket creation:

@Override
public Map<String, String> toAuditMap() {
Map<String, String> auditMap = new LinkedHashMap<>();
auditMap.put(OzoneConsts.VOLUME, this.volumeName);
auditMap.put(OzoneConsts.BUCKET, this.bucketName);
auditMap.put(OzoneConsts.BUCKET_LAYOUT, String.valueOf(this.bucketLayout));
auditMap.put(OzoneConsts.GDPR_FLAG,
getMetadata().get(OzoneConsts.GDPR_FLAG));
auditMap.put(OzoneConsts.ACLS,
(this.acls != null) ? this.acls.toString() : null);
auditMap.put(OzoneConsts.IS_VERSION_ENABLED,
String.valueOf(this.isVersionEnabled));
auditMap.put(OzoneConsts.STORAGE_TYPE,
(this.storageType != null) ? this.storageType.name() : null);
auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime));
auditMap.put(OzoneConsts.BUCKET_ENCRYPTION_KEY,
(bekInfo != null) ? bekInfo.getKeyName() : null);
auditMap.put(OzoneConsts.MODIFICATION_TIME,
String.valueOf(this.modificationTime));
if (isLink()) {
auditMap.put(OzoneConsts.SOURCE_VOLUME, sourceVolume);
auditMap.put(OzoneConsts.SOURCE_BUCKET, sourceBucket);
}
auditMap.put(OzoneConsts.USED_BYTES, String.valueOf(this.usedBytes));
auditMap.put(OzoneConsts.USED_NAMESPACE,
String.valueOf(this.usedNamespace));
return auditMap;
}

Quotas and default replication config need to be added (please double-check I did not miss anything).

It is OK to make these additional changes in follow-up PR if you prefer, since the task is currently specific to property update operations.

@tanvipenumudy
Copy link
Contributor Author

On another look, similar change is needed in OmBucketInfo, which is used as source of audit data during bucket creation
Quotas and default replication config need to be added

Thank you @adoroszlai and @myskov for the reviews. I have filed a ticket: HDDS-10475 for tracking the suggested change!

@adoroszlai adoroszlai merged commit 418528a into apache:master Mar 6, 2024
@adoroszlai
Copy link
Contributor

Thanks @tanvipenumudy for the patch, @myskov for the review.

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 15, 2024
apache#6329)

(cherry picked from commit 418528a)
Change-Id: Ic89d26f3ab4a75508547b590f190a58f675738c3
myskov pushed a commit to myskov/ozone that referenced this pull request Apr 3, 2024
myskov pushed a commit to myskov/ozone that referenced this pull request Apr 3, 2024
myskov pushed a commit to myskov/ozone that referenced this pull request Apr 3, 2024
myskov pushed a commit to myskov/ozone that referenced this pull request Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants