Extend sssctl to manage cached GPOs

[scabrero]

Extends the sssctl tool to show, list, remove and purge cached GPOs.

Fixes #4523

[thalman]

@scabrero, I reviewd the changes, looks great! Thanks for the effort.

Would you be so kind and update also sss_cache man page? (src/man/sss_cache.8.xml)

Tomáš

[thalman]

@scabrero, I reviewd the changes, looks great! Thanks for the effort.

Would you be so kind and update also sss_cache man page? (src/man/sss_cache.8.xml)

Sorry, mixed up sssctl and sss_cache. Ignore my comment, please.

T.

[thalman]

LGTM, works as expected, ACK

[alexey-tikhonov]

Hi @scabrero,

could you please add RN?
See https://github.com/SSSD/sssd/blob/master/.git-commit-template#L7 for a hint.

[scabrero]

Hi @scabrero,

could you please add RN? See https://github.com/SSSD/sssd/blob/master/.git-commit-template#L7 for a hint.

Hi @alexey-tikhonov, I added an empty commit with the :relnote: tag. If you prefer to amend the latest patch just let me know and I will push again.

Thanks for the reviews!

[sumit-bose]

Hi,

thank you for the patches, in general I'm fine but please my in-line comments.

Some general comments. sssctl gpo-show and sssctl gpo.remove require that the domain name must be given, e.g

sssctl gpo-show 'Default Domain Policy'@samba.test

or

sssctl gpo-show -g {31B2F340-016D-11D2-945F-00C04FB984F9}@samba.test

which imo should be mentioned in the --help output because it might be unexpected.

Is there is reason you are using the description attribute to store the display name and not the name attribute? The benefit would be that name is already indexed. Additionally, gpoGUID isn't indexed as well and since there are now searches by GUID it might be worth to add an index for gpoGUID as well?

bye,
Sumit

[scabrero]

Hi,

I have addressed all suggestions and pushed an update with the following changes:

Hi Sumit,

Some general comments. sssctl gpo-show and sssctl gpo.remove require that the domain name must be given
Ok, fixed.

Is there is reason you are using the description attribute to store the display name and not the name attribute? The benefit would be that name is already indexed.

Because I saw that get_attr_name() in sssctl_cache.c checks dom->fqnames to build a FQDN, so I used SYSDB_DESCRIPTION to keep it separated from use_fully_qualified_names. I agree with the index reasoning so I have changed it to use SYSDB_NAME but kept a separate handler for it.

Additionally, gpoGUID isn't indexed as well and since there are now searches by GUID it might be worth to add an index for gpoGUID as well?

Yes, and I made it case-insensitive for searches.

Summary of changes:

• Switch to Sumit's suggested fix for GPO referral processing when subdomain connection is not yet initialized.
• Store the AD displayName attribute in SYSDB_NAME instead of SYSDB_DESCRIPTION.
• Bump sysdb version to 0.24: Index gpoGUID and make it case-insensitive on searches.
• Add new parameter to show extended help for sssctl subcommands
• Add extended help for gpo-show and gpo-remove subcommands.

[sumit-bose]

Hi,

thank you for your patience, I have no further comments, ACK.

bye,
Sumit

[pbrezina]

• master
  • 54179a0 - SSSCTL: Add the new cached GPOs management commands to release notes
  • c5b16ee - SSSCTL: Add gpo-purge command
  • afee68b - SSSCTL: Add sssctl gpo-remove command
  • be73599 - SYSDB: Add a function to delete GPO entry by GPO GUID
  • 6dc9166 - SSSCTL: Add sssctl gpo-list command
  • 18a17bc - SSSCTL: Add gpo-show command
  • 095e31e - SSSCTL: Prepare for extended help in subcommands
  • cf59da1 - SYSDB: Add new index for gpoGUID and make searches on it case insensitive
  • 66fd8a0 - SYSDB: Always canonicalize GPO guid
  • 3580134 - SYSDB: Store the GPO's filesystem path in sysdb entry
  • 568ca5d - SYSDB: Store GPO's displayName in sysdb
  • e169277 - GPO: Fetch the GPO's displayName attribute
  • 9926067 - SYSDB: Add sysdb_gpos_base_dn()
  • 98efb5e - GPO: Remove unused local variable
  • 738bb53 - GPO: Defer SMB server choice until id connection established when processing referrals