Skip to content

Commit

Permalink
[DPE-4199] Fix user secrets inherit integration hub secrets when usin…
Browse files Browse the repository at this point in the history
…g add-config (#92)
  • Loading branch information
welpaolo authored Apr 24, 2024
1 parent cac3677 commit bc7c422
Show file tree
Hide file tree
Showing 13 changed files with 237 additions and 178 deletions.
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 <[email protected]>"
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

0 comments on commit bc7c422

Please sign in to comment.