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.

Sign up for GitHub

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

Fix installed plugins to use a dedicated dir to prevent naming conflict #347

Merged
merged 13 commits into from
Nov 21, 2022
Merged

Fix installed plugins to use a dedicated dir to prevent naming conflict #347

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
480c48f
Updating install_plugins and settings file with installed_plugins path
PhillypHenning Nov 4, 2022
7ddc4c1
Updating deploy_plugins with installed_plugins
PhillypHenning Nov 4, 2022
3474eaf
adding mask None check
PhillypHenning Nov 4, 2022
b6fe04d
linting and formatting
PhillypHenning Nov 4, 2022
ea06f1c
final formatting
PhillypHenning Nov 4, 2022
c32c755
Removing Mask Check (moving to its own PR)
PhillypHenning Nov 8, 2022
f1aa65e
removing configuration option `installed_plugins_dir`
PhillypHenning Nov 14, 2022
9db46d5
Removing `installed_plugins_dir` references from doc
PhillypHenning Nov 15, 2022
d71a36e
Updating `X_plugins_dir` usage to use the `BITOPS_installed_plugins_d…
PhillypHenning Nov 15, 2022
33f36b3
Removing unneeded assignment + fixing ENV vars
PhillypHenning Nov 21, 2022
ba7a921
Making `BITOPS_installed_plugins_dir` a constant
PhillypHenning Nov 21, 2022
10349d1
Merge branch 'main' into installed_plugins
PhillypHenning Nov 21, 2022
cae0b0b
formatting and linting updates
PhillypHenning Nov 21, 2022
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: 4 additions & 0 deletions bitops.schema.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ bitops:
type: string
description: "Path to log folder"

installed_plugins_dir:
type: sring
description: "Name of the directory where plugins will be installed"
PhillypHenning marked this conversation as resolved.
Show resolved Hide resolved

plugins:
type: object
properties:
Expand Down
3 changes: 2 additions & 1 deletion docs/plugins.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ bitops:
err: bitops.logs # error logs filename
path: /var/logs/bitops # path to log folder
default_folder: _default
plugins:
installed_plugins_dir: installed-plugins
PhillypHenning marked this conversation as resolved.
Show resolved Hide resolved
plugins:
aws:
source: https://github.com/bitops-plugins/aws
terraform:
Expand Down
1 change: 1 addition & 0 deletions docs/tool-configuration/bitops.schema.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ bitops:
fail_fast: <value>
logging:
level: <value>
installed_plugins_dir: <value>
PhillypHenning marked this conversation as resolved.
Show resolved Hide resolved
plugins:
plugin_seq:
- <value>
Expand Down
16 changes: 13 additions & 3 deletions scripts/plugins/deploy_plugins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import sys
import stat
import tempfile
from distutils.dir_util import copy_tree
from munch import DefaultMunch
Expand All @@ -14,6 +15,8 @@
BITOPS_ENV_environment,
BITOPS_default_folder,
BITOPS_timeout,
BITOPS_plugin_dir,
BITOPS_installed_plugins_dir,
)
from .logging import logger

Expand Down Expand Up @@ -45,7 +48,8 @@ def deploy_plugins(): # pylint: disable=too-many-locals,too-many-branches,too-m

bitops_dir = "/opt/bitops"
bitops_deployment_dir = "/opt/bitops_deployment/"
bitops_plugins_dir = bitops_dir + "/scripts/plugins/"
bitops_plugins_dir = BITOPS_plugin_dir
Copy link
Member

@arm4b arm4b Nov 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This variable is used only once. We could remove the unneeded variable assignment from this line:

Suggested change
bitops_plugins_dir = BITOPS_plugin_dir

and use the original BITOPS_plugin_dir var later below:

bitops/scripts/plugins/deploy_plugins.py

Line 113 in d71a36e

\n\t BITOPS_PLUGIN_DIR: [{bitops_plugins_dir}] \

that will keep vars a bit cleaner in this function.

bitops_installed_plugins_dir = BITOPS_installed_plugins_dir
PhillypHenning marked this conversation as resolved.
Show resolved Hide resolved

bitops_root_dir = temp_dir

Expand All @@ -62,7 +66,7 @@ def deploy_plugins(): # pylint: disable=too-many-locals,too-many-branches,too-m
os.environ["BITOPS_ENVROOT"] = bitops_operations_dir
os.environ["BITOPS_DIR"] = bitops_dir
os.environ["BITOPS_SCRIPTS_DIR"] = bitops_scripts_dir
os.environ["BITOPS_PLUGINS_DIR"] = bitops_plugins_dir
os.environ["BITOPS_PLUGINS_DIR"] = bitops_installed_plugins_dir
os.environ["BITOPS_FAIL_FAST"] = str(BITOPS_fast_fail_mode)
os.environ["BITOPS_KUBE_CONFIG_FILE"] = f"{temp_dir}/.kube/config"
os.environ["BITOPS_DEFAULT_ROOT_DIR"] = BITOPS_default_folder
Expand Down Expand Up @@ -121,7 +125,9 @@ def deploy_plugins(): # pylint: disable=too-many-locals,too-many-branches,too-m
plugin_name = bitops_deployment_configuration[deployment].plugin

# Set plugin vars
plugin_dir = bitops_plugins_dir + plugin_name # Sourced from BitOps Core + plugin install
plugin_dir = (
bitops_installed_plugins_dir + plugin_name
) # Sourced from BitOps Core + plugin install
opsrepo_environment_dir = (
bitops_operations_dir + "/" + deployment
) # Sourced from Operations repo
Expand Down Expand Up @@ -225,6 +231,10 @@ def deploy_plugins(): # pylint: disable=too-many-locals,too-many-branches,too-m
logger.warning("setting null value for stack_action....")
stack_action = ""

# Ensure execute bit is present on deploy script
st = os.stat(plugin_deploy_script_path)
os.chmod(plugin_deploy_script_path, st.st_mode | stat.S_IEXEC)

# Adding print env logging
bitops_env_vars = [item for item in os.environ if "BITOPS_" in item]
bitops_env_vars.sort()
Expand Down
18 changes: 11 additions & 7 deletions scripts/plugins/install_plugins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from .utilities import run_cmd
from .doc import get_doc
from .logging import logger
from .settings import BITOPS_config_yaml, BITOPS_plugin_dir
from .settings import BITOPS_config_yaml, BITOPS_installed_plugins_dir


# TODO: Refactor this function. Fix pylint R0914: Too many local variables (22/15) (too-many-locals)
Expand All @@ -32,7 +32,7 @@ def install_plugins(): # pylint: disable=too-many-locals,too-many-statements
bitops_build_configuration.bitops.plugins, None
)

plugin_dir = BITOPS_plugin_dir
installed_plugins_dir = BITOPS_installed_plugins_dir
PhillypHenning marked this conversation as resolved.
Show resolved Hide resolved
plugin_list = list(bitops_plugins_configuration)
# Loop through plugins and clone
for plugin_config in bitops_plugins_configuration:
Expand Down Expand Up @@ -75,17 +75,19 @@ def install_plugins(): # pylint: disable=too-many-locals,too-many-statements
try:
# Non-Entry default
if plugin_branch == "latest" and plugin_tag == "main":
git.Repo.clone_from(plugin_source, plugin_dir + plugin_config)
git.Repo.clone_from(plugin_source, installed_plugins_dir + plugin_config)

# If the plugin branch and tag are specified, default to branch
elif plugin_branch is not None and plugin_tag is not None:
git.Repo.clone_from(plugin_source, plugin_dir + plugin_config, branch=plugin_branch)
git.Repo.clone_from(
plugin_source, installed_plugins_dir + plugin_config, branch=plugin_branch
)

else:
plugin_pull_branch = plugin_tag if plugin_branch is None else plugin_branch
git.Repo.clone_from(
plugin_source,
plugin_dir + plugin_config,
installed_plugins_dir + plugin_config,
branch=plugin_pull_branch,
)

Expand All @@ -102,7 +104,7 @@ def install_plugins(): # pylint: disable=too-many-locals,too-many-statements
# ~#~#~#~#~#~#~#~#~#~#~#~#~#

# Once the plugin is cloned, begin using its config + schema
plugin_configuration_path = plugin_dir + plugin_config + "/plugin.config.yaml"
plugin_configuration_path = installed_plugins_dir + plugin_config + "/plugin.config.yaml"
logger.info(f"plugin_configuration_path ==>[{plugin_configuration_path}]")
try:
with open(plugin_configuration_path, "r", encoding="utf8") as stream:
Expand Down Expand Up @@ -161,7 +163,9 @@ def install_plugins(): # pylint: disable=too-many-locals,too-many-statements
sys.exit(10)

# install plugin dependencies (install.sh)
plugin_install_script_path = plugin_dir + plugin_config + f"/{plugin_install_script}"
plugin_install_script_path = (
installed_plugins_dir + plugin_config + f"/{plugin_install_script}"
)

logger.info(
f"\n\n\n~#~#~#~INSTALLING PLUGIN [{plugin_config}]~#~#~#~ \
Expand Down
3 changes: 3 additions & 0 deletions scripts/plugins/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@ def mask_message(message):
"""
if message is None:
return message
if BITOPS_logging_masks is None:
return message
PhillypHenning marked this conversation as resolved.
Show resolved Hide resolved

res_str = message

for config_item in BITOPS_logging_masks:
# TODO: use a library here?
res_str = re.sub(rf"{config_item.search}", config_item.replace, str(res_str))
Expand Down
8 changes: 8 additions & 0 deletions scripts/plugins/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
BITOPS_ENV_run_mode = os.environ.get("BITOPS_MODE")
BITOPS_ENV_logging_level = os.environ.get("BITOPS_LOGGING_LEVEL")
BITOPS_ENV_plugin_dir = os.environ.get("BITOPS_PLUGIN_DIR")
BITOPS_ENV_installed_plugin_dir = os.environ.get("BITOPS_INSTALLED_PLUGIN_DIR")
Copy link
Member

Choose a reason for hiding this comment

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

If we agreed to not expose the configuration for this setting via bitops.config.yaml, can we exclude setting it from the ENV var as a source too?

That's again for the reasons of simplicity and avoiding exposing configuration that's not needed by the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right but plugin scripts directly use this env var ...

Copy link
Member

Choose a reason for hiding this comment

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

The exporting for the plugins looks fine.

But here we're importing the ENV var from the user's side. This could be removed as we don't want users to override this setting.

Copy link
Member

Choose a reason for hiding this comment

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

This would make BITOPS_INSTALLED_PLUGIN_DIR = /opt/bitops/scripts/installed_plugins/ a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha 👍 Will fix this after lunch


BITOPS_ENV_default_folder = os.environ.get("BITOPS_DEFAULT_FOLDER")
# v2.0.0: Fallback to 'ENVIRONMENT' in case when 'BITOPS_ENVIRONMENT' is not set
Expand Down Expand Up @@ -106,6 +107,13 @@
else "/opt/bitops/scripts/plugins/"
)

BITOPS_installed_plugins_dir = (
BITOPS_ENV_installed_plugin_dir
if BITOPS_ENV_installed_plugin_dir is not None
else bitops_build_configuration.bitops.installed_plugins_dir
if bitops_build_configuration.bitops.installed_plugins_dir is not None
else "/opt/bitops/scripts/installed_plugins/"
)

BITOPS_default_folder = (
BITOPS_ENV_default_folder
Expand Down