Revert "[pdh] Make the admin share configurable (#5485)"

This reverts commit 1abcf9a.
DataDog · Jan 27, 2020 · ec01a32 · ec01a32
1 parent 249c79b
commit ec01a32
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 89 deletions.
diff --git a/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py b/datadog_checks_base/datadog_checks/base/checks/win/winpdh_base.py
@@ -13,9 +13,6 @@
     from .winpdh_stub import WinPDHCounter, DATA_TYPE_INT, DATA_TYPE_DOUBLE
 
 
-RESOURCETYPE_ANY = 0
-DEFAULT_SHARE = 'c$'
-
 int_types = ["int", "long", "uint"]
 
 double_types = ["double", "float"]
@@ -55,7 +52,10 @@ def __init__(self, name, init_config, agentConfig, instances, counter_list):
 
                         username = instance.get('username')
                         password = instance.get('password')
-                        nr = self._get_netresource(remote_machine)
+                        nr = win32wnet.NETRESOURCE()
+                        nr.lpRemoteName = r"\\%s\c$" % remote_machine
+                        nr.dwType = 0
+                        nr.lpLocalName = None
                         win32wnet.WNetAddConnection2(nr, password, username, 0)
 
                     except Exception as e:
@@ -105,42 +105,6 @@ def __init__(self, name, init_config, agentConfig, instances, counter_list):
         if key is None or not self._metrics.get(key):
             raise AttributeError('No valid counters to collect')
 
-    def _get_netresource(self, remote_machine):
-        # To connect you have to use the name of the server followed by an optional administrative share.
-        # Administrative shares are hidden network shares created that allow system administrators to have remote access
-        # to every disk volume on a network-connected system.
-        # These shares may not be permanently deleted but may be disabled.
-        # Administrative shares cannot be accessed by users without administrative privileges.
-        #
-        # This page explains how to enable them: https://www.wintips.org/how-to-enable-admin-shares-windows-7/
-        #
-        # The administrative share can be:
-        # * A disk volume like c$
-        # * admin$: The folder in which Windows is installed
-        # * fax$: The folder in which faxed pages and cover pages are cached
-        # * ipc$: Area used for interprocess communication and is not part of the file system.
-        # * print$: Virtual folder that contains a representation of the installed printers
-        # * Domain controller shares: Windows creates two domain controller specific shares called sysvol and netlogon
-        #   which do not have $ appended to their names.
-        # * Empty string: No admin share specified
-        administrative_share = self.instance.get('admin_share', DEFAULT_SHARE)
-
-        nr = win32wnet.NETRESOURCE()
-
-        # Specifies the network resource to connect to.
-        nr.lpRemoteName = r"\\{}\{}".format(remote_machine, administrative_share).rstrip('\\')
-
-        # The type of network resource to connect to.
-        #
-        # Although this member is required, its information may be ignored by the network service provider.
-        nr.dwType = RESOURCETYPE_ANY
-
-        # Specifies the name of a local device to redirect, such as "F:" or "LPT1".
-        # If the string is empty, NULL, it connects to the network resource without redirecting a local device.
-        nr.lpLocalName = None
-
-        return nr
-
     def check(self, instance):
         self.log.debug("PDHBaseCheck: check()")
         key = hash_mutable(instance)

diff --git a/datadog_checks_base/tests/test_pdhbasecheck.py b/datadog_checks_base/tests/test_pdhbasecheck.py
@@ -2,8 +2,6 @@
 # All rights reserved
 # Licensed under a 3-clause BSD style license (see LICENSE)
 
-import copy
-
 import pytest
 
 from .utils import requires_windows
@@ -28,16 +26,6 @@
     ["Processor", "1", "% Processor Time", "test.processor_time_1", "gauge"],
 ]
 
-PARTIAL_COUNTER_LIST = [
-    ["NTDS", None, "LDAP Client Sessions", "active_directory.ldap.client_sessions", "gauge"],
-    ["NTDS", None, "LDAP Bind Time", "active_directory.ldap.bind_time", "gauge"],
-    ["NTDS", None, "LDAP Successful Binds/sec", "active_directory.ldap.successful_binds_persec", "gauge"],
-    ["NTDS", None, "LDAP Searches/sec", "active_directory.ldap.searches_persec", "gauge"],
-    # these two don't exist
-    ["NTDS", None, "Kerberos Authentications/sec", "active_directory.kerberos.auths_persec", "gauge"],
-    ["NTDS", None, "NTLM Authentications/sec", "active_directory.ntlm.auths_persec", "gauge"],
-]
-
 
 @requires_windows
 def test_single_instance_counter(aggregator, pdh_mocks_fixture):  # noqa F811
@@ -81,41 +69,22 @@ def test_multi_instance_counter_specific_instances(aggregator, pdh_mocks_fixture
 
 @requires_windows
 def test_returns_partial_metrics(aggregator, pdh_mocks_fixture):  # noqa F811
+    COUNTER_LIST = [
+        ["NTDS", None, "LDAP Client Sessions", "active_directory.ldap.client_sessions", "gauge"],
+        ["NTDS", None, "LDAP Bind Time", "active_directory.ldap.bind_time", "gauge"],
+        ["NTDS", None, "LDAP Successful Binds/sec", "active_directory.ldap.successful_binds_persec", "gauge"],
+        ["NTDS", None, "LDAP Searches/sec", "active_directory.ldap.searches_persec", "gauge"],
+        # these two don't exist
+        ["NTDS", None, "Kerberos Authentications/sec", "active_directory.kerberos.auths_persec", "gauge"],
+        ["NTDS", None, "NTLM Authentications/sec", "active_directory.ntlm.auths_persec", "gauge"],
+    ]
     initialize_pdh_tests()
     instance = DEFAULT_INSTANCE
-    c = PDHBaseCheck("testcheck", {}, {}, [instance], PARTIAL_COUNTER_LIST)
+    c = PDHBaseCheck("testcheck", {}, {}, [instance], COUNTER_LIST)
     c.check(instance)
 
     aggregator.assert_metric("active_directory.ldap.client_sessions", tags=None, count=1)
     aggregator.assert_metric("active_directory.ldap.bind_time", tags=None, count=1)
     aggregator.assert_metric("active_directory.ldap.successful_binds_persec", tags=None, count=1)
     aggregator.assert_metric("active_directory.ldap.searches_persec", tags=None, count=1)
     assert aggregator.metrics_asserted_pct == 100.0
-
-
-@requires_windows
-def test_default_admin_share():
-    initialize_pdh_tests()
-    c = PDHBaseCheck("testcheck", {}, {}, [DEFAULT_INSTANCE], SINGLE_INSTANCE_COUNTER)
-    nr = c._get_netresource('1.1.1.1')
-    assert nr.lpRemoteName == '\\\\1.1.1.1\\c$'
-
-
-@requires_windows
-def test_custom_admin_share():
-    initialize_pdh_tests()
-    instance = copy.deepcopy(DEFAULT_INSTANCE)
-    instance['admin_share'] = 'ipc$'
-    c = PDHBaseCheck("testcheck", {}, {}, [instance], SINGLE_INSTANCE_COUNTER)
-    nr = c._get_netresource('1.2.3.4')
-    assert nr.lpRemoteName == '\\\\1.2.3.4\\ipc$'
-
-
-@requires_windows
-def test_no_admin_share():
-    initialize_pdh_tests()
-    instance = copy.deepcopy(DEFAULT_INSTANCE)
-    instance['admin_share'] = ''
-    c = PDHBaseCheck("testcheck", {}, {}, [instance], SINGLE_INSTANCE_COUNTER)
-    nr = c._get_netresource('1.2.3.4')
-    assert nr.lpRemoteName == '\\\\1.2.3.4'
diff --git a/pdh_check/datadog_checks/pdh_check/data/conf.yaml.example b/pdh_check/datadog_checks/pdh_check/data/conf.yaml.example
@@ -18,14 +18,6 @@ instances:
     #
     # password: <PASSWORD>
 
-    ## @param admin_share - string - optional - default: c$
-    ## The administrative share to connect to. Can be a drive or the `ipc$` share if available.
-    ## Note that to be able to connect to remote hosts the administrative share needs to be enabled and the
-    ## user needs network administrator permissions
-    ## If the remote machine doesn't expose any, set this to the empty string `""` to connect without an admin share. 
-    #
-    # admin_share: c$
-
     ## The following example fetches the number of processes and users:
     ##
     ## - countersetname: Processor