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.

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

[DPE-4199] Fix user secrets inherit integration hub secrets when using add-config #92

Merged
merged 2 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ ignore_missing_imports = true

[tool.poetry]
name = "spark8t"
version = "0.0.7"
version = "0.0.8"
description = "This project provides some utilities function and CLI commands to run Spark on K8s."
authors = [
"Canonical Data Platform <data-platform@lists.launchpad.net>"
Expand Down
263 changes: 126 additions & 137 deletions requirements.txt

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions spark8t/cli/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,16 @@ def add_logging_arguments(parser: ArgumentParser) -> ArgumentParser:
return parser


def add_ignore_integrator_hub(parser: ArgumentParser) -> ArgumentParser:
def add_ignore_integration_hub(parser: ArgumentParser) -> ArgumentParser:
"""
Add option to exclude the configuration provided by the Spark Integrator Hub
Add option to exclude the configuration provided by the Spark Integration Hub

:param parser: Input parser to decorate with parsing support for logging args.
"""
parser.add_argument(
"--ignore-integrator-hub",
"--ignore-integration-hub",
action="store_true",
help="Ignore the configuration provided by Spark Integrator Hub Charm.",
help="Ignore the configuration provided by Spark Integration Hub Charm.",
)

return parser
Expand Down
8 changes: 4 additions & 4 deletions spark8t/cli/pyspark.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from spark8t.cli.params import (
add_config_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
add_logging_arguments,
defaults,
get_kube_interface,
Expand Down Expand Up @@ -43,8 +43,8 @@ def main(args: Namespace, logger: Logger):
else PrimaryAccountNotFound()
)

if args.ignore_integrator_hub:
service_account.integrator_hub_confs = PropertyFile.empty()
if args.ignore_integration_hub:
service_account.integration_hub_confs = PropertyFile.empty()

SparkInterface(
service_account=service_account,
Expand All @@ -60,7 +60,7 @@ def main(args: Namespace, logger: Logger):
k8s_parser,
spark_user_parser,
add_config_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
]
).parse_known_args()

Expand Down
12 changes: 6 additions & 6 deletions spark8t/cli/service_account_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ def create_service_account_registry_parser(parser: ArgumentParser):
[spark_user_parser],
subparsers.add_parser(Actions.GET_CONFIG.value, parents=[base_parser]),
).add_argument(
"--ignore-integrator-hub",
"--ignore-integration-hub",
action="store_true",
help="Boolean to ignore Spark Integrator Hub generated options.",
help="Boolean to ignore Spark Integration Hub generated options.",
)
# subparser for sa-conf-del
parse_arguments_with(
Expand Down Expand Up @@ -137,7 +137,7 @@ def main(args: Namespace, logger: Logger):
raise AccountNotFound(input_service_account.id)

account_configuration = (
service_account_in_registry.configurations
service_account_in_registry.extra_confs
+ (
PropertyFile.read(args.properties_file)
if args.properties_file is not None
Expand All @@ -158,7 +158,7 @@ def main(args: Namespace, logger: Logger):

registry.set_configurations(
input_service_account.id,
service_account_in_registry.configurations.remove(args.conf),
service_account_in_registry.extra_confs.remove(args.conf),
)

elif args.action == Actions.GET_CONFIG:
Expand All @@ -169,8 +169,8 @@ def main(args: Namespace, logger: Logger):
if maybe_service_account is None:
raise AccountNotFound(input_service_account.id)

if args.ignore_integrator_hub:
maybe_service_account.integrator_hub_confs = PropertyFile.empty()
if args.ignore_integration_hub:
maybe_service_account.integration_hub_confs = PropertyFile.empty()
maybe_service_account.configurations.log(print)

elif args.action == Actions.CLEAR_CONFIG:
Expand Down
8 changes: 4 additions & 4 deletions spark8t/cli/spark_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from spark8t.cli.params import (
add_config_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
add_logging_arguments,
defaults,
get_kube_interface,
Expand Down Expand Up @@ -43,8 +43,8 @@ def main(args: Namespace, logger: Logger):
else PrimaryAccountNotFound()
)

if args.ignore_integrator_hub:
service_account.integrator_hub_confs = PropertyFile.empty()
if args.ignore_integration_hub:
service_account.integration_hub_confs = PropertyFile.empty()

SparkInterface(
service_account=service_account,
Expand All @@ -60,7 +60,7 @@ def main(args: Namespace, logger: Logger):
k8s_parser,
spark_user_parser,
add_config_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
]
).parse_known_args()

Expand Down
8 changes: 4 additions & 4 deletions spark8t/cli/spark_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from spark8t.cli.params import (
add_config_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
add_logging_arguments,
defaults,
get_kube_interface,
Expand Down Expand Up @@ -43,8 +43,8 @@ def main(args: Namespace, logger: Logger):
else PrimaryAccountNotFound()
)

if args.ignore_integrator_hub:
service_account.integrator_hub_confs = PropertyFile.empty()
if args.ignore_integration_hub:
service_account.integration_hub_confs = PropertyFile.empty()

SparkInterface(
service_account=service_account,
Expand All @@ -60,7 +60,7 @@ def main(args: Namespace, logger: Logger):
k8s_parser,
spark_user_parser,
add_config_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
]
).parse_known_args()

Expand Down
8 changes: 4 additions & 4 deletions spark8t/cli/spark_submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from spark8t.cli.params import (
add_config_arguments,
add_deploy_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
add_logging_arguments,
defaults,
get_kube_interface,
Expand Down Expand Up @@ -44,8 +44,8 @@ def main(args: Namespace, logger: Logger):
else PrimaryAccountNotFound()
)

if args.ignore_integrator_hub:
service_account.integrator_hub_confs = PropertyFile.empty()
if args.ignore_integration_hub:
service_account.integration_hub_confs = PropertyFile.empty()

SparkInterface(
service_account=service_account,
Expand All @@ -67,7 +67,7 @@ def main(args: Namespace, logger: Logger):
spark_user_parser,
add_deploy_arguments,
add_config_arguments,
add_ignore_integrator_hub,
add_ignore_integration_hub,
]
).parse_known_args()

Expand Down
4 changes: 2 additions & 2 deletions spark8t/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ class ServiceAccount:
api_server: str
primary: bool = False
extra_confs: PropertyFile = PropertyFile.empty()
integrator_hub_confs: PropertyFile = PropertyFile.empty()
integration_hub_confs: PropertyFile = PropertyFile.empty()

@property
def id(self):
Expand All @@ -319,7 +319,7 @@ def _k8s_configurations(self):
@property
def configurations(self) -> PropertyFile:
"""Return the service account configuration, associated to a given spark service account."""
return self.extra_confs + self.integrator_hub_confs + self._k8s_configurations
return self.extra_confs + self.integration_hub_confs + self._k8s_configurations


class KubernetesResourceType(str, Enum):
Expand Down
2 changes: 1 addition & 1 deletion spark8t/literals.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
MANAGED_BY_LABELNAME = "app.kubernetes.io/managed-by"
PRIMARY_LABELNAME = "app.kubernetes.io/spark8t-primary"
SPARK8S_LABEL = "spark8t"
HUB_LABEL = "integrator-hub-conf"
HUB_LABEL = "integration-hub-conf"
2 changes: 1 addition & 1 deletion spark8t/resources/templates/role_yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,6 @@ rules:
resources:
- secrets
resourceNames:
- integrator-hub-conf-{{username}}
- integration-hub-conf-{{username}}
verbs:
- get
8 changes: 4 additions & 4 deletions spark8t/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ def _get_secret_name(name):
return f"{SPARK8S_LABEL}-sa-conf-{name}"

@staticmethod
def _get_integrator_hub_secret_name(name):
def _get_integration_hub_secret_name(name):
return f"{HUB_LABEL}-{name}"

def _retrieve_secret_configurations(
Expand All @@ -1183,7 +1183,7 @@ def _build_service_account_from_raw(self, metadata: Dict[str, Any]):
primary = PRIMARY_LABELNAME in metadata["labels"]

account_secret_name = self._get_secret_name(name)
integrator_hub_secret_name = self._get_integrator_hub_secret_name(name)
integration_hub_secret_name = self._get_integration_hub_secret_name(name)

return ServiceAccount(
name=name,
Expand All @@ -1193,8 +1193,8 @@ def _build_service_account_from_raw(self, metadata: Dict[str, Any]):
extra_confs=self._retrieve_secret_configurations(
name, namespace, account_secret_name
),
integrator_hub_confs=self._retrieve_secret_configurations(
name, namespace, integrator_hub_secret_name
integration_hub_confs=self._retrieve_secret_configurations(
name, namespace, integration_hub_secret_name
),
)

Expand Down
82 changes: 76 additions & 6 deletions tests/integration/test_cli_service_accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ def test_service_account_get_config(service_account, backend, request):
}
assert actual_configs == expected_configs

# add integrator hub secret for the test service account
# add integration hub secret for the test service account
secret_name = f"{HUB_LABEL}-{username}"

property_file = PropertyFile({"key": "value"})
Expand Down Expand Up @@ -589,7 +589,7 @@ def test_service_account_get_config(service_account, backend, request):
KubernetesResourceType.SECRET_GENERIC, secret_name, namespace
)

# check that integrator hub config is there
# check that integration hub config is there
# Get the default configs created with a service account
stdout, stderr, ret_code = run_service_account_registry(
"get-config",
Expand Down Expand Up @@ -617,14 +617,14 @@ def test_service_account_get_config(service_account, backend, request):
namespace,
"--backend",
backend,
"--ignore-integrator-hub",
"--ignore-integration-hub",
)
actual_configs = set(stdout.splitlines())
assert actual_configs == expected_configs


@pytest.mark.parametrize("backend", VALID_BACKENDS)
def test_service_account_add_config(service_account, backend):
def test_service_account_add_config(service_account, backend, request):
"""Test addition of service account config using the CLI.

Use a fixture that creates temporary service account, add new config,
Expand All @@ -645,6 +645,34 @@ def test_service_account_add_config(service_account, backend):
)
original_configs = set(stdout.splitlines())

# add integration hub secret for the test service account
secret_name = f"{HUB_LABEL}-{username}"

property_file = PropertyFile({"key": "value"})

kubeinterface = request.getfixturevalue("kubeinterface")

with umask_named_temporary_file(
mode="w",
prefix="spark-dynamic-conf-k8s-",
suffix=".conf",
dir=os.path.expanduser("~"),
) as t:
property_file.write(t.file)

t.flush()

kubeinterface.create(
KubernetesResourceType.SECRET_GENERIC,
secret_name,
namespace=namespace,
**{"from-env-file": str(t.name)},
)

assert kubeinterface.exists(
KubernetesResourceType.SECRET_GENERIC, secret_name, namespace
)

config_to_add = "foo=bar"

# Add a few new configs
Expand All @@ -669,7 +697,7 @@ def test_service_account_add_config(service_account, backend):
namespace,
"--backend",
backend,
"--ignore-integrator-hub",
"--ignore-integration-hub",
)
updated_configs = set(stdout.splitlines())

Expand All @@ -680,7 +708,7 @@ def test_service_account_add_config(service_account, backend):


@pytest.mark.parametrize("backend", VALID_BACKENDS)
def test_service_account_remove_config(service_account, backend):
def test_service_account_remove_config(service_account, backend, request):
"""Test removal of service account config using the CLI.

Use a fixture that creates temporary service account, add new config,
Expand All @@ -690,6 +718,34 @@ def test_service_account_remove_config(service_account, backend):
"""
username, namespace = service_account

# add integration hub secret for the test service account
secret_name = f"{HUB_LABEL}-{username}"

property_file = PropertyFile({"key": "value"})

kubeinterface = request.getfixturevalue("kubeinterface")

with umask_named_temporary_file(
mode="w",
prefix="spark-dynamic-conf-k8s-",
suffix=".conf",
dir=os.path.expanduser("~"),
) as t:
property_file.write(t.file)

t.flush()

kubeinterface.create(
KubernetesResourceType.SECRET_GENERIC,
secret_name,
namespace=namespace,
**{"from-env-file": str(t.name)},
)

assert kubeinterface.exists(
KubernetesResourceType.SECRET_GENERIC, secret_name, namespace
)

config_to_add = "foo=bar"

# Add new configs
Expand Down Expand Up @@ -736,6 +792,20 @@ def test_service_account_remove_config(service_account, backend):
new_configs = set(stdout.splitlines())
assert config_to_add not in new_configs

# Ensure that the added configs have been successfully created
stdout, stderr, ret_code = run_service_account_registry(
"get-config",
"--username",
username,
"--namespace",
namespace,
"--backend",
backend,
"--ignore-integration-hub",
)
conf = set(stdout.splitlines())
assert "key=value" not in conf


@pytest.mark.parametrize("backend", VALID_BACKENDS)
def test_service_account_clear_config(service_account, backend):
Expand Down